memsize
ActivePublic
Actions

Authored by br on Dec 15 2014, 2:21 PM.

Tags

None

Referenced Files

	F58649: memsize
	Dec 16 2014, 5:30 PM

	F58648: memsize
	Dec 16 2014, 5:20 PM

	F58606: memsize
	Dec 16 2014, 1:11 PM

	F58250: memsize
	Dec 15 2014, 5:40 PM

	F58225: memsize
	Dec 15 2014, 2:21 PM

Subscribers

brooks

	Index: sys/mips/beri/beri_machdep.c
	===================================================================
	--- sys/mips/beri/beri_machdep.c (revision 275828)
	+++ sys/mips/beri/beri_machdep.c (working copy)
	@@ -88,6 +88,11 @@
	mips_init(void)
	{
	int i;
	+#ifdef FDT
	+ struct mem_region mr[FDT_MEM_REGIONS];
	+ int mr_cnt, val;
	+ int j;
	+#endif

	for (i = 0; i < 10; i++) {
	phys_avail[i] = 0;
	@@ -102,6 +107,29 @@

	physmem = realmem;

	+#ifdef FDT
	+ if (fdt_get_mem_regions(mr, &mr_cnt, &val) == 0) {
	+
	+ physmem = btoc(val);
	+
	+ KASSERT((phys_avail[0] >= mr[0].mr_start) && \
	+ (phys_avail[0] < (mr[0].mr_start + mr[0].mr_size)),
	+ ("First region is not within FDT memory range"));
	+
	+ /* Limit size of the first region */
	+ phys_avail[1] = MIN(mr[0].mr_size, ctob(realmem));
	+ dump_avail[1] = phys_avail[1];
	+
	+ /* Add the rest of regions */
	+ for (i = 1, j = 2; i < mr_cnt; i++, j+=2) {
	+ phys_avail[j] = mr[i].mr_start;
	+ phys_avail[j+1] = mr[i].mr_size;
	+ dump_avail[j] = phys_avail[j];
	+ dump_avail[j+1] = phys_avail[j+1];
	+ }
	+ }
	+#endif
	+
	init_param1();
	init_param2(physmem);
	mips_cpu_init();

	Index: sys/boot/fdt/dts/mips/beri-netfpga.dts
	===================================================================
	--- sys/boot/fdt/dts/mips/beri-netfpga.dts (revision 275828)
	+++ sys/boot/fdt/dts/mips/beri-netfpga.dts (working copy)
	@@ -82,6 +82,11 @@
	*/
	};

	+ memory {
	+ device_type = "memory";
	+ reg = <0x0 0x0FFFFFFF>; // ~256M at 0x0
	+ };
	+
	soc {
	#address-cells = <1>;
	#size-cells = <1>;
	@@ -94,11 +99,6 @@
	compatible = "simple-bus", "mips,mips4k";
	ranges = <>;

	- memory {
	- device_type = "memory";
	- reg = <0x0 0x0FFFFFFF>; // ~256M at 0x0
	- };
	-
	beripic: beripic@7f804000 {
	compatible = "sri-cambridge,beri-pic";
	interrupt-controller;
	Index: sys/boot/fdt/dts/mips/beri-sim.dts
	===================================================================
	--- sys/boot/fdt/dts/mips/beri-sim.dts (revision 275828)
	+++ sys/boot/fdt/dts/mips/beri-sim.dts (working copy)
	@@ -80,6 +80,11 @@
	*/
	};

	+ memory {
	+ device_type = "memory";
	+ reg = <0x0 0x4000000>; // 64M at 0x0
	+ };
	+
	soc {
	#address-cells = <1>;
	#size-cells = <1>;
	@@ -92,11 +97,6 @@
	compatible = "simple-bus", "mips,mips4k";
	ranges = <>;

	- memory {
	- device_type = "memory";
	- reg = <0x0 0x4000000>; // 64M at 0x0
	- };
	-
	beripic0: beripic@7f804000 {
	compatible = "sri-cambridge,beri-pic";
	interrupt-controller;
	Index: sys/boot/fdt/dts/mips/beripad-de4.dts
	===================================================================
	--- sys/boot/fdt/dts/mips/beripad-de4.dts (revision 275828)
	+++ sys/boot/fdt/dts/mips/beripad-de4.dts (working copy)
	@@ -80,6 +80,11 @@
	*/
	};

	+ memory {
	+ device_type = "memory";
	+ reg = <0x0 0x40000000>; // 1G at 0x0
	+ };
	+
	soc {
	#address-cells = <1>;
	#size-cells = <1>;
	@@ -92,11 +97,6 @@
	compatible = "simple-bus", "mips,mips4k";
	ranges = <>;

	- memory {
	- device_type = "memory";
	- reg = <0x0 0x40000000>; // 1G at 0x0
	- };
	-
	beripic0: beripic@7f804000 {
	compatible = "sri-cambridge,beri-pic";
	interrupt-controller;

Event Timeline

br edited the content of this paste. (Show Details)Dec 15 2014, 2:21 PM

br changed the title of this paste from untitled to memsize.

br updated the paste's language from autodetect to c.

It looks like the memsize is cumulative so if there are multiple regions I think that won't be what the kernel expects. We'll need to make sure we have correct values in the FDT for the DE4 before we turn this on as I believe it will set memsize too large currently.

br edited the content of this paste. (Show Details)Dec 15 2014, 5:40 PM

I have updated the paste. We can fill phys_avail by using all the memory regions provided in FDT, but seems we still need to ignore the start of first region, as it is dynamic

So I'm not sure how is better

Setting regions from the FDT does seem like the right thing to do when they are available. I'd include an XXX comment that the first region is assumed to include the kernel as there's no guarantee that's the case in the FDT.

I still don't think updating memsize to the cumulative memory size is safe with multiple regions. I think you either want to set it to rm[0].mr_start + mr[0].mr_size or only use the value if there is only a single region. Arguably, if you're using the regions from FDT you don't need to touch it anyway. Also, I'd avoid increasing memory size or the length of the first region based on the FDT.

I still don't think updating memsize to the cumulative memory size is safe with multiple regions. I think you either want to set it to rm[0].mr_start + mr[0].mr_size or only use the value if there is only a single region. Arguably, if you're using the regions from FDT you don't need to touch it anyway.

The idea here is to overwrite memsize in case of using FDT only, after overwriting it only is used for printf() purposes. So it seems it is correct to show to user cumulative memory size. May be some additional checks required to not use memsize in case of FDT.

Also, I'd avoid increasing memory size or the length of the first region based on the FDT.

Sure.

memsize is used to control size of the first region after the point where you're setting it in:

realmem = btoc(memsize);
mips_init();

br edited the content of this paste. (Show Details)Dec 16 2014, 1:11 PM

as I see in init_param2() the cumulative physmem is fine.

so is the last patch fine or I still don't understand ?

This looks good. I would like to see an assert that phys_avail[0] is within the first memory region. Otherwise things are going to go badly wrong if a bad FDT is provide where that isn't true.

Added ASSERT

br edited the content of this paste. (Show Details)Dec 16 2014, 5:30 PM

Looks ready to go.

I would also move memory node in FDT to the root as fdt_get_mem_regions looks to the root node

memsizeActivePublicActions

Event Timeline

memsize
ActivePublic
Actions