| Message ID | 20180323081857.57138-6-icenowy@aosc.io |
|---|---|
| State | Changes Requested |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
| Series | Add 3GiB DRAM support to 64-bit Allwinner SoCs | expand |
On Fri, Mar 23, 2018 at 04:18:56PM +0800, Icenowy Zheng wrote: > The get_ram_size() function in U-Boot can only deal with memory size > smaller than 2GiB. To enable the support of 3GiB DRAM on newer 64-bit > SoCs, an alternative way to detect DRAM size is needed. Why not just fixing get_ram_size then? Maxime
于 2018年3月23日 GMT+08:00 下午5:40:41, Maxime Ripard <maxime.ripard@bootlin.com> 写到: >On Fri, Mar 23, 2018 at 04:18:56PM +0800, Icenowy Zheng wrote: >> The get_ram_size() function in U-Boot can only deal with memory size >> smaller than 2GiB. To enable the support of 3GiB DRAM on newer 64-bit >> SoCs, an alternative way to detect DRAM size is needed. > >Why not just fixing get_ram_size then? Even if it's fixed it won't support 3GiB DRAM at all. > >Maxime
On Fri, Mar 23, 2018 at 05:41:43PM +0800, Icenowy Zheng wrote: > > > 于 2018年3月23日 GMT+08:00 下午5:40:41, Maxime Ripard <maxime.ripard@bootlin.com> 写到: > >On Fri, Mar 23, 2018 at 04:18:56PM +0800, Icenowy Zheng wrote: > >> The get_ram_size() function in U-Boot can only deal with memory size > >> smaller than 2GiB. To enable the support of 3GiB DRAM on newer 64-bit > >> SoCs, an alternative way to detect DRAM size is needed. > > > >Why not just fixing get_ram_size then? > > Even if it's fixed it won't support 3GiB DRAM at all. Why? Maxime
于 2018年3月26日 GMT+08:00 下午3:06:33, Maxime Ripard <maxime.ripard@bootlin.com> 写到: >On Fri, Mar 23, 2018 at 05:41:43PM +0800, Icenowy Zheng wrote: >> >> >> 于 2018年3月23日 GMT+08:00 下午5:40:41, Maxime Ripard ><maxime.ripard@bootlin.com> 写到: >> >On Fri, Mar 23, 2018 at 04:18:56PM +0800, Icenowy Zheng wrote: >> >> The get_ram_size() function in U-Boot can only deal with memory >size >> >> smaller than 2GiB. To enable the support of 3GiB DRAM on newer >64-bit >> >> SoCs, an alternative way to detect DRAM size is needed. >> > >> >Why not just fixing get_ram_size then? >> >> Even if it's fixed it won't support 3GiB DRAM at all. > >Why? It has an assumption that the size is pow of 2. > >Maxime
On Mon, Mar 26, 2018 at 03:11:04PM +0800, Icenowy Zheng wrote: > > > 于 2018年3月26日 GMT+08:00 下午3:06:33, Maxime Ripard <maxime.ripard@bootlin.com> 写到: > >On Fri, Mar 23, 2018 at 05:41:43PM +0800, Icenowy Zheng wrote: > >> > >> > >> 于 2018年3月23日 GMT+08:00 下午5:40:41, Maxime Ripard > ><maxime.ripard@bootlin.com> 写到: > >> >On Fri, Mar 23, 2018 at 04:18:56PM +0800, Icenowy Zheng wrote: > >> >> The get_ram_size() function in U-Boot can only deal with memory > >size > >> >> smaller than 2GiB. To enable the support of 3GiB DRAM on newer > >64-bit > >> >> SoCs, an alternative way to detect DRAM size is needed. > >> > > >> >Why not just fixing get_ram_size then? > >> > >> Even if it's fixed it won't support 3GiB DRAM at all. > > > >Why? > > It has an assumption that the size is pow of 2. I guess this would be fixable too? (or one could create a variant without that assumption). Maxime
于 2018年3月28日 GMT+08:00 下午7:28:07, Maxime Ripard <maxime.ripard@bootlin.com> 写到: >On Mon, Mar 26, 2018 at 03:11:04PM +0800, Icenowy Zheng wrote: >> >> >> 于 2018年3月26日 GMT+08:00 下午3:06:33, Maxime Ripard ><maxime.ripard@bootlin.com> 写到: >> >On Fri, Mar 23, 2018 at 05:41:43PM +0800, Icenowy Zheng wrote: >> >> >> >> >> >> 于 2018年3月23日 GMT+08:00 下午5:40:41, Maxime Ripard >> ><maxime.ripard@bootlin.com> 写到: >> >> >On Fri, Mar 23, 2018 at 04:18:56PM +0800, Icenowy Zheng wrote: >> >> >> The get_ram_size() function in U-Boot can only deal with memory >> >size >> >> >> smaller than 2GiB. To enable the support of 3GiB DRAM on newer >> >64-bit >> >> >> SoCs, an alternative way to detect DRAM size is needed. >> >> > >> >> >Why not just fixing get_ram_size then? >> >> >> >> Even if it's fixed it won't support 3GiB DRAM at all. >> > >> >Why? >> >> It has an assumption that the size is pow of 2. > >I guess this would be fixable too? (or one could create a variant >without that assumption). I don't think its principle allows such kind of fix, as it just checks writing then reading at some offset that is pow if 2. For hacking it, see my implementation in v1, which assumes the only size supported bigger than 2GiB is 3GiB (which is acceptable on sunxi, but might not work on other platforms). As Andre said, that function has another big problem -- it detects memory with writing to it. This is risky. > >Maxime
On Wed, Mar 28, 2018 at 07:31:51PM +0800, Icenowy Zheng wrote: > 于 2018年3月28日 GMT+08:00 下午7:28:07, Maxime Ripard <maxime.ripard@bootlin.com> 写到: > >On Mon, Mar 26, 2018 at 03:11:04PM +0800, Icenowy Zheng wrote: > >> > >> > >> 于 2018年3月26日 GMT+08:00 下午3:06:33, Maxime Ripard > ><maxime.ripard@bootlin.com> 写到: > >> >On Fri, Mar 23, 2018 at 05:41:43PM +0800, Icenowy Zheng wrote: > >> >> > >> >> > >> >> 于 2018年3月23日 GMT+08:00 下午5:40:41, Maxime Ripard > >> ><maxime.ripard@bootlin.com> 写到: > >> >> >On Fri, Mar 23, 2018 at 04:18:56PM +0800, Icenowy Zheng wrote: > >> >> >> The get_ram_size() function in U-Boot can only deal with memory > >> >size > >> >> >> smaller than 2GiB. To enable the support of 3GiB DRAM on newer > >> >64-bit > >> >> >> SoCs, an alternative way to detect DRAM size is needed. > >> >> > > >> >> >Why not just fixing get_ram_size then? > >> >> > >> >> Even if it's fixed it won't support 3GiB DRAM at all. > >> > > >> >Why? > >> > >> It has an assumption that the size is pow of 2. > > > >I guess this would be fixable too? (or one could create a variant > >without that assumption). > > I don't think its principle allows such kind of fix, as it just > checks writing then reading at some offset that is pow if 2. You could do have a bunch of algorithm actually. One would be to write the address in memory and try to detect where exactly it starts to loop. You could do a bisection in the opposite direction once you settled for the upper limit (so you would have for example a workable 2G, a non-workable 4G, and then you try intervals that you always divide by two, so testing then 3G (that works), then halfway between 3G and 4G, etc. > For hacking it, see my implementation in v1, which assumes the > only size supported bigger than 2GiB is 3GiB (which is > acceptable on sunxi, but might not work on other platforms). > > As Andre said, that function has another big problem -- it detects > memory with writing to it. This is risky. How is it risky when it's done by the SPL? Maxime
Hi, On 29/03/18 10:37, Maxime Ripard wrote: > On Wed, Mar 28, 2018 at 07:31:51PM +0800, Icenowy Zheng wrote: >> 于 2018年3月28日 GMT+08:00 下午7:28:07, Maxime Ripard <maxime.ripard@bootlin.com> 写到: >>> On Mon, Mar 26, 2018 at 03:11:04PM +0800, Icenowy Zheng wrote: >>>> >>>> >>>> 于 2018年3月26日 GMT+08:00 下午3:06:33, Maxime Ripard >>> <maxime.ripard@bootlin.com> 写到: >>>>> On Fri, Mar 23, 2018 at 05:41:43PM +0800, Icenowy Zheng wrote: >>>>>> >>>>>> >>>>>> 于 2018年3月23日 GMT+08:00 下午5:40:41, Maxime Ripard >>>>> <maxime.ripard@bootlin.com> 写到: >>>>>>> On Fri, Mar 23, 2018 at 04:18:56PM +0800, Icenowy Zheng wrote: >>>>>>>> The get_ram_size() function in U-Boot can only deal with memory >>>>> size >>>>>>>> smaller than 2GiB. To enable the support of 3GiB DRAM on newer >>>>> 64-bit >>>>>>>> SoCs, an alternative way to detect DRAM size is needed. >>>>>>> >>>>>>> Why not just fixing get_ram_size then? >>>>>> >>>>>> Even if it's fixed it won't support 3GiB DRAM at all. >>>>> >>>>> Why? >>>> >>>> It has an assumption that the size is pow of 2. >>> >>> I guess this would be fixable too? (or one could create a variant >>> without that assumption). >> >> I don't think its principle allows such kind of fix, as it just >> checks writing then reading at some offset that is pow if 2. > > You could do have a bunch of algorithm actually. One would be to write > the address in memory and try to detect where exactly it starts to > loop. > > You could do a bisection in the opposite direction once you settled > for the upper limit (so you would have for example a workable 2G, a > non-workable 4G, and then you try intervals that you always divide by > two, so testing then 3G (that works), then halfway between 3G and 4G, > etc. > >> For hacking it, see my implementation in v1, which assumes the >> only size supported bigger than 2GiB is 3GiB (which is >> acceptable on sunxi, but might not work on other platforms). >> >> As Andre said, that function has another big problem -- it detects >> memory with writing to it. This is risky. > > How is it risky when it's done by the SPL? Originally that was my confusion as well: It's not the SPL calling that function. The DRAM controller init function in there knows very precisely how much DRAM we have, but we don't communicate this to U-Boot proper. So U-Boot *proper* goes ahead and probes the DRAM. This means it could step into secure memory, for instance. On sunxi64 we have the ATF running between SPL and U-Boot, also all kind of secure payloads could already have been registered. So I wonder if it would be easier to somehow pass on this *one* word of information between SPL and U-Boot proper to avoid calling this function altogether? Cheers, Andre.
On Thu, Mar 29, 2018 at 01:21:38PM +0100, Andre Przywara wrote: > Hi, > > On 29/03/18 10:37, Maxime Ripard wrote: > > On Wed, Mar 28, 2018 at 07:31:51PM +0800, Icenowy Zheng wrote: > >> 于 2018年3月28日 GMT+08:00 下午7:28:07, Maxime Ripard <maxime.ripard@bootlin.com> 写到: > >>> On Mon, Mar 26, 2018 at 03:11:04PM +0800, Icenowy Zheng wrote: > >>>> > >>>> > >>>> 于 2018年3月26日 GMT+08:00 下午3:06:33, Maxime Ripard > >>> <maxime.ripard@bootlin.com> 写到: > >>>>> On Fri, Mar 23, 2018 at 05:41:43PM +0800, Icenowy Zheng wrote: > >>>>>> > >>>>>> > >>>>>> 于 2018年3月23日 GMT+08:00 下午5:40:41, Maxime Ripard > >>>>> <maxime.ripard@bootlin.com> 写到: > >>>>>>> On Fri, Mar 23, 2018 at 04:18:56PM +0800, Icenowy Zheng wrote: > >>>>>>>> The get_ram_size() function in U-Boot can only deal with memory > >>>>> size > >>>>>>>> smaller than 2GiB. To enable the support of 3GiB DRAM on newer > >>>>> 64-bit > >>>>>>>> SoCs, an alternative way to detect DRAM size is needed. > >>>>>>> > >>>>>>> Why not just fixing get_ram_size then? > >>>>>> > >>>>>> Even if it's fixed it won't support 3GiB DRAM at all. > >>>>> > >>>>> Why? > >>>> > >>>> It has an assumption that the size is pow of 2. > >>> > >>> I guess this would be fixable too? (or one could create a variant > >>> without that assumption). > >> > >> I don't think its principle allows such kind of fix, as it just > >> checks writing then reading at some offset that is pow if 2. > > > > You could do have a bunch of algorithm actually. One would be to write > > the address in memory and try to detect where exactly it starts to > > loop. > > > > You could do a bisection in the opposite direction once you settled > > for the upper limit (so you would have for example a workable 2G, a > > non-workable 4G, and then you try intervals that you always divide by > > two, so testing then 3G (that works), then halfway between 3G and 4G, > > etc. > > > >> For hacking it, see my implementation in v1, which assumes the > >> only size supported bigger than 2GiB is 3GiB (which is > >> acceptable on sunxi, but might not work on other platforms). > >> > >> As Andre said, that function has another big problem -- it detects > >> memory with writing to it. This is risky. > > > > How is it risky when it's done by the SPL? > > Originally that was my confusion as well: It's not the SPL calling that > function. The DRAM controller init function in there knows very > precisely how much DRAM we have, but we don't communicate this to U-Boot > proper. So U-Boot *proper* goes ahead and probes the DRAM. This means it > could step into secure memory, for instance. On sunxi64 we have the ATF > running between SPL and U-Boot, also all kind of secure payloads could > already have been registered. > So I wonder if it would be easier to somehow pass on this *one* word of > information between SPL and U-Boot proper to avoid calling this function > altogether? That would definitely make sense yes. Maxime
Hi, On 03/04/18 10:29, Maxime Ripard wrote: > On Thu, Mar 29, 2018 at 01:21:38PM +0100, Andre Przywara wrote: >> Hi, >> >> On 29/03/18 10:37, Maxime Ripard wrote: >>> On Wed, Mar 28, 2018 at 07:31:51PM +0800, Icenowy Zheng wrote: >>>> 于 2018年3月28日 GMT+08:00 下午7:28:07, Maxime Ripard <maxime.ripard@bootlin.com> 写到: >>>>> On Mon, Mar 26, 2018 at 03:11:04PM +0800, Icenowy Zheng wrote: >>>>>> >>>>>> >>>>>> 于 2018年3月26日 GMT+08:00 下午3:06:33, Maxime Ripard >>>>> <maxime.ripard@bootlin.com> 写到: >>>>>>> On Fri, Mar 23, 2018 at 05:41:43PM +0800, Icenowy Zheng wrote: >>>>>>>> >>>>>>>> >>>>>>>> 于 2018年3月23日 GMT+08:00 下午5:40:41, Maxime Ripard >>>>>>> <maxime.ripard@bootlin.com> 写到: >>>>>>>>> On Fri, Mar 23, 2018 at 04:18:56PM +0800, Icenowy Zheng wrote: >>>>>>>>>> The get_ram_size() function in U-Boot can only deal with memory >>>>>>> size >>>>>>>>>> smaller than 2GiB. To enable the support of 3GiB DRAM on newer >>>>>>> 64-bit >>>>>>>>>> SoCs, an alternative way to detect DRAM size is needed. >>>>>>>>> >>>>>>>>> Why not just fixing get_ram_size then? >>>>>>>> >>>>>>>> Even if it's fixed it won't support 3GiB DRAM at all. >>>>>>> >>>>>>> Why? >>>>>> >>>>>> It has an assumption that the size is pow of 2. >>>>> >>>>> I guess this would be fixable too? (or one could create a variant >>>>> without that assumption). >>>> >>>> I don't think its principle allows such kind of fix, as it just >>>> checks writing then reading at some offset that is pow if 2. >>> >>> You could do have a bunch of algorithm actually. One would be to write >>> the address in memory and try to detect where exactly it starts to >>> loop. >>> >>> You could do a bisection in the opposite direction once you settled >>> for the upper limit (so you would have for example a workable 2G, a >>> non-workable 4G, and then you try intervals that you always divide by >>> two, so testing then 3G (that works), then halfway between 3G and 4G, >>> etc. >>> >>>> For hacking it, see my implementation in v1, which assumes the >>>> only size supported bigger than 2GiB is 3GiB (which is >>>> acceptable on sunxi, but might not work on other platforms). >>>> >>>> As Andre said, that function has another big problem -- it detects >>>> memory with writing to it. This is risky. >>> >>> How is it risky when it's done by the SPL? >> >> Originally that was my confusion as well: It's not the SPL calling that >> function. The DRAM controller init function in there knows very >> precisely how much DRAM we have, but we don't communicate this to U-Boot >> proper. So U-Boot *proper* goes ahead and probes the DRAM. This means it >> could step into secure memory, for instance. On sunxi64 we have the ATF >> running between SPL and U-Boot, also all kind of secure payloads could >> already have been registered. >> So I wonder if it would be easier to somehow pass on this *one* word of >> information between SPL and U-Boot proper to avoid calling this function >> altogether? > > That would definitely make sense yes. So since the SPL loads the DT anyway (from the FIT image) and puts it at the end of the U-Boot (proper) binary, wouldn't it be the easiest to just patch the actual DRAM size in there? IIRC we don't have any FDT write code in the SPL at the moment, and pulling it in would probably push it over the edge again, but: We should be able to somewhat short-cut it, if it's just about to actually *patch* an existing value, as of: - make sure we have a memory node and a reg property in the DT - in the SPL learn the fdt offset of that reg property - <hack> patch in the memory size into the second word </hack> - teach U-Boot proper to read the memory size from the DT - optionally look at #address-cells and #size-cells to make this "second word hack" more robust This could actually be a rather generic patch, just with some "avoid libfdt write library" hack to address our size issue. Maybe we already have something like that for some platform (Rockchip comes to mind?) How does that sound? Cheers, Andre.
于 2018年4月3日 GMT+08:00 下午6:13:17, Andre Przywara <andre.przywara@arm.com> 写到: >Hi, > >On 03/04/18 10:29, Maxime Ripard wrote: >> On Thu, Mar 29, 2018 at 01:21:38PM +0100, Andre Przywara wrote: >>> Hi, >>> >>> On 29/03/18 10:37, Maxime Ripard wrote: >>>> On Wed, Mar 28, 2018 at 07:31:51PM +0800, Icenowy Zheng wrote: >>>>> 于 2018年3月28日 GMT+08:00 下午7:28:07, Maxime Ripard ><maxime.ripard@bootlin.com> 写到: >>>>>> On Mon, Mar 26, 2018 at 03:11:04PM +0800, Icenowy Zheng wrote: >>>>>>> >>>>>>> >>>>>>> 于 2018年3月26日 GMT+08:00 下午3:06:33, Maxime Ripard >>>>>> <maxime.ripard@bootlin.com> 写到: >>>>>>>> On Fri, Mar 23, 2018 at 05:41:43PM +0800, Icenowy Zheng wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> 于 2018年3月23日 GMT+08:00 下午5:40:41, Maxime Ripard >>>>>>>> <maxime.ripard@bootlin.com> 写到: >>>>>>>>>> On Fri, Mar 23, 2018 at 04:18:56PM +0800, Icenowy Zheng >wrote: >>>>>>>>>>> The get_ram_size() function in U-Boot can only deal with >memory >>>>>>>> size >>>>>>>>>>> smaller than 2GiB. To enable the support of 3GiB DRAM on >newer >>>>>>>> 64-bit >>>>>>>>>>> SoCs, an alternative way to detect DRAM size is needed. >>>>>>>>>> >>>>>>>>>> Why not just fixing get_ram_size then? >>>>>>>>> >>>>>>>>> Even if it's fixed it won't support 3GiB DRAM at all. >>>>>>>> >>>>>>>> Why? >>>>>>> >>>>>>> It has an assumption that the size is pow of 2. >>>>>> >>>>>> I guess this would be fixable too? (or one could create a variant >>>>>> without that assumption). >>>>> >>>>> I don't think its principle allows such kind of fix, as it just >>>>> checks writing then reading at some offset that is pow if 2. >>>> >>>> You could do have a bunch of algorithm actually. One would be to >write >>>> the address in memory and try to detect where exactly it starts to >>>> loop. >>>> >>>> You could do a bisection in the opposite direction once you settled >>>> for the upper limit (so you would have for example a workable 2G, a >>>> non-workable 4G, and then you try intervals that you always divide >by >>>> two, so testing then 3G (that works), then halfway between 3G and >4G, >>>> etc. >>>> >>>>> For hacking it, see my implementation in v1, which assumes the >>>>> only size supported bigger than 2GiB is 3GiB (which is >>>>> acceptable on sunxi, but might not work on other platforms). >>>>> >>>>> As Andre said, that function has another big problem -- it detects >>>>> memory with writing to it. This is risky. >>>> >>>> How is it risky when it's done by the SPL? >>> >>> Originally that was my confusion as well: It's not the SPL calling >that >>> function. The DRAM controller init function in there knows very >>> precisely how much DRAM we have, but we don't communicate this to >U-Boot >>> proper. So U-Boot *proper* goes ahead and probes the DRAM. This >means it >>> could step into secure memory, for instance. On sunxi64 we have the >ATF >>> running between SPL and U-Boot, also all kind of secure payloads >could >>> already have been registered. >>> So I wonder if it would be easier to somehow pass on this *one* word >of >>> information between SPL and U-Boot proper to avoid calling this >function >>> altogether? >> >> That would definitely make sense yes. > >So since the SPL loads the DT anyway (from the FIT image) and puts it >at >the end of the U-Boot (proper) binary, wouldn't it be the easiest to >just patch the actual DRAM size in there? >IIRC we don't have any FDT write code in the SPL at the moment, and >pulling it in would probably push it over the edge again, but: >We should be able to somewhat short-cut it, if it's just about to >actually *patch* an existing value, as of: >- make sure we have a memory node and a reg property in the DT >- in the SPL learn the fdt offset of that reg property >- <hack> patch in the memory size into the second word </hack> >- teach U-Boot proper to read the memory size from the DT >- optionally look at #address-cells and #size-cells to make this >"second >word hack" more robust > >This could actually be a rather generic patch, just with some "avoid >libfdt write library" hack to address our size issue. Maybe we already >have something like that for some platform (Rockchip comes to mind?) Rockchip recalculates the memory size with the parameters in GRF when running the main U-Boot, so both TPL/SPL and miniloader can load mainline U-Boot. > >How does that sound? > >Cheers, >Andre.
> On 3 Apr 2018, at 12:39, Icenowy Zheng <icenowy@aosc.io> wrote: > > > > 于 2018年4月3日 GMT+08:00 下午6:13:17, Andre Przywara <andre.przywara@arm.com <mailto:andre.przywara@arm.com>> 写到: >> Hi, >> >> On 03/04/18 10:29, Maxime Ripard wrote: >>> On Thu, Mar 29, 2018 at 01:21:38PM +0100, Andre Przywara wrote: >>>> Hi, >>>> >>>> On 29/03/18 10:37, Maxime Ripard wrote: >>>>> On Wed, Mar 28, 2018 at 07:31:51PM +0800, Icenowy Zheng wrote: >>>>>> 于 2018年3月28日 GMT+08:00 下午7:28:07, Maxime Ripard >> <maxime.ripard@bootlin.com> 写到: >>>>>>> On Mon, Mar 26, 2018 at 03:11:04PM +0800, Icenowy Zheng wrote: >>>>>>>> >>>>>>>> >>>>>>>> 于 2018年3月26日 GMT+08:00 下午3:06:33, Maxime Ripard >>>>>>> <maxime.ripard@bootlin.com> 写到: >>>>>>>>> On Fri, Mar 23, 2018 at 05:41:43PM +0800, Icenowy Zheng wrote: >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> 于 2018年3月23日 GMT+08:00 下午5:40:41, Maxime Ripard >>>>>>>>> <maxime.ripard@bootlin.com> 写到: >>>>>>>>>>> On Fri, Mar 23, 2018 at 04:18:56PM +0800, Icenowy Zheng >> wrote: >>>>>>>>>>>> The get_ram_size() function in U-Boot can only deal with >> memory >>>>>>>>> size >>>>>>>>>>>> smaller than 2GiB. To enable the support of 3GiB DRAM on >> newer >>>>>>>>> 64-bit >>>>>>>>>>>> SoCs, an alternative way to detect DRAM size is needed. >>>>>>>>>>> >>>>>>>>>>> Why not just fixing get_ram_size then? >>>>>>>>>> >>>>>>>>>> Even if it's fixed it won't support 3GiB DRAM at all. >>>>>>>>> >>>>>>>>> Why? >>>>>>>> >>>>>>>> It has an assumption that the size is pow of 2. >>>>>>> >>>>>>> I guess this would be fixable too? (or one could create a variant >>>>>>> without that assumption). >>>>>> >>>>>> I don't think its principle allows such kind of fix, as it just >>>>>> checks writing then reading at some offset that is pow if 2. >>>>> >>>>> You could do have a bunch of algorithm actually. One would be to >> write >>>>> the address in memory and try to detect where exactly it starts to >>>>> loop. >>>>> >>>>> You could do a bisection in the opposite direction once you settled >>>>> for the upper limit (so you would have for example a workable 2G, a >>>>> non-workable 4G, and then you try intervals that you always divide >> by >>>>> two, so testing then 3G (that works), then halfway between 3G and >> 4G, >>>>> etc. >>>>> >>>>>> For hacking it, see my implementation in v1, which assumes the >>>>>> only size supported bigger than 2GiB is 3GiB (which is >>>>>> acceptable on sunxi, but might not work on other platforms). >>>>>> >>>>>> As Andre said, that function has another big problem -- it detects >>>>>> memory with writing to it. This is risky. >>>>> >>>>> How is it risky when it's done by the SPL? >>>> >>>> Originally that was my confusion as well: It's not the SPL calling >> that >>>> function. The DRAM controller init function in there knows very >>>> precisely how much DRAM we have, but we don't communicate this to >> U-Boot >>>> proper. So U-Boot *proper* goes ahead and probes the DRAM. This >> means it >>>> could step into secure memory, for instance. On sunxi64 we have the >> ATF >>>> running between SPL and U-Boot, also all kind of secure payloads >> could >>>> already have been registered. >>>> So I wonder if it would be easier to somehow pass on this *one* word >> of >>>> information between SPL and U-Boot proper to avoid calling this >> function >>>> altogether? >>> >>> That would definitely make sense yes. >> >> So since the SPL loads the DT anyway (from the FIT image) and puts it >> at >> the end of the U-Boot (proper) binary, wouldn't it be the easiest to >> just patch the actual DRAM size in there? >> IIRC we don't have any FDT write code in the SPL at the moment, and >> pulling it in would probably push it over the edge again, but: >> We should be able to somewhat short-cut it, if it's just about to >> actually *patch* an existing value, as of: >> - make sure we have a memory node and a reg property in the DT >> - in the SPL learn the fdt offset of that reg property >> - <hack> patch in the memory size into the second word </hack> >> - teach U-Boot proper to read the memory size from the DT >> - optionally look at #address-cells and #size-cells to make this >> "second >> word hack" more robust >> >> This could actually be a rather generic patch, just with some "avoid >> libfdt write library" hack to address our size issue. Maybe we already >> have something like that for some platform (Rockchip comes to mind?) > > Rockchip recalculates the memory size with the parameters > in GRF when running the main U-Boot, so both TPL/SPL > and miniloader can load mainline U-Boot. More accurately, the GRF currently is used to store an encoded DRAM configuration and the next stage can rely on this being there. If the memory is initialised by miniloader, U-Boot reads the GRF and calculates the memory size from that. If U-Boot sets up DRAM, then it also populates the GRF registers. With more and more SOCs moving away from miniloader, there’s a good chance that this will change at some point in the future and the GRF may not necessarily be populated anymore. —Philipp.
On Tue, Apr 03, 2018 at 11:13:17AM +0100, Andre Przywara wrote: > >>>> For hacking it, see my implementation in v1, which assumes the > >>>> only size supported bigger than 2GiB is 3GiB (which is > >>>> acceptable on sunxi, but might not work on other platforms). > >>>> > >>>> As Andre said, that function has another big problem -- it detects > >>>> memory with writing to it. This is risky. > >>> > >>> How is it risky when it's done by the SPL? > >> > >> Originally that was my confusion as well: It's not the SPL calling that > >> function. The DRAM controller init function in there knows very > >> precisely how much DRAM we have, but we don't communicate this to U-Boot > >> proper. So U-Boot *proper* goes ahead and probes the DRAM. This means it > >> could step into secure memory, for instance. On sunxi64 we have the ATF > >> running between SPL and U-Boot, also all kind of secure payloads could > >> already have been registered. > >> So I wonder if it would be easier to somehow pass on this *one* word of > >> information between SPL and U-Boot proper to avoid calling this function > >> altogether? > > > > That would definitely make sense yes. > > So since the SPL loads the DT anyway (from the FIT image) and puts it at > the end of the U-Boot (proper) binary, wouldn't it be the easiest to > just patch the actual DRAM size in there? > IIRC we don't have any FDT write code in the SPL at the moment, and > pulling it in would probably push it over the edge again, but: That assumes that you are loading a FIT image, which might or might not be the case, and on most arm32 chips, most likely won't. I guess we'd need to find another way (put some information in an SRAM somewhere?) to try to do that for all variants. Maxime
Hi, On 03/04/18 12:34, Maxime Ripard wrote: > On Tue, Apr 03, 2018 at 11:13:17AM +0100, Andre Przywara wrote: >>>>>> For hacking it, see my implementation in v1, which assumes the >>>>>> only size supported bigger than 2GiB is 3GiB (which is >>>>>> acceptable on sunxi, but might not work on other platforms). >>>>>> >>>>>> As Andre said, that function has another big problem -- it detects >>>>>> memory with writing to it. This is risky. >>>>> >>>>> How is it risky when it's done by the SPL? >>>> >>>> Originally that was my confusion as well: It's not the SPL calling that >>>> function. The DRAM controller init function in there knows very >>>> precisely how much DRAM we have, but we don't communicate this to U-Boot >>>> proper. So U-Boot *proper* goes ahead and probes the DRAM. This means it >>>> could step into secure memory, for instance. On sunxi64 we have the ATF >>>> running between SPL and U-Boot, also all kind of secure payloads could >>>> already have been registered. >>>> So I wonder if it would be easier to somehow pass on this *one* word of >>>> information between SPL and U-Boot proper to avoid calling this function >>>> altogether? >>> >>> That would definitely make sense yes. >> >> So since the SPL loads the DT anyway (from the FIT image) and puts it at >> the end of the U-Boot (proper) binary, wouldn't it be the easiest to >> just patch the actual DRAM size in there? >> IIRC we don't have any FDT write code in the SPL at the moment, and >> pulling it in would probably push it over the edge again, but: > > That assumes that you are loading a FIT image, which might or might > not be the case, and on most arm32 chips, most likely won't. That's true, but my understanding is that this >4GB is only relevant for 64-bit SoCs, where we mandate FIT images, don't we? > I guess we'd need to find another way (put some information in an > SRAM somewhere?) to try to do that for all variants. Oh, so your aim at getting rid of the call to the memory probing function at all? I was just assuming that we change it for the purpose of this 3GB support, which would be 64-bit only? Cheers, Andre
于 2018年4月3日 GMT+08:00 下午7:41:41, Andre Przywara <andre.przywara@arm.com> 写到: >Hi, > >On 03/04/18 12:34, Maxime Ripard wrote: >> On Tue, Apr 03, 2018 at 11:13:17AM +0100, Andre Przywara wrote: >>>>>>> For hacking it, see my implementation in v1, which assumes the >>>>>>> only size supported bigger than 2GiB is 3GiB (which is >>>>>>> acceptable on sunxi, but might not work on other platforms). >>>>>>> >>>>>>> As Andre said, that function has another big problem -- it >detects >>>>>>> memory with writing to it. This is risky. >>>>>> >>>>>> How is it risky when it's done by the SPL? >>>>> >>>>> Originally that was my confusion as well: It's not the SPL calling >that >>>>> function. The DRAM controller init function in there knows very >>>>> precisely how much DRAM we have, but we don't communicate this to >U-Boot >>>>> proper. So U-Boot *proper* goes ahead and probes the DRAM. This >means it >>>>> could step into secure memory, for instance. On sunxi64 we have >the ATF >>>>> running between SPL and U-Boot, also all kind of secure payloads >could >>>>> already have been registered. >>>>> So I wonder if it would be easier to somehow pass on this *one* >word of >>>>> information between SPL and U-Boot proper to avoid calling this >function >>>>> altogether? >>>> >>>> That would definitely make sense yes. >>> >>> So since the SPL loads the DT anyway (from the FIT image) and puts >it at >>> the end of the U-Boot (proper) binary, wouldn't it be the easiest to >>> just patch the actual DRAM size in there? >>> IIRC we don't have any FDT write code in the SPL at the moment, and >>> pulling it in would probably push it over the edge again, but: >> >> That assumes that you are loading a FIT image, which might or might >> not be the case, and on most arm32 chips, most likely won't. > >That's true, but my understanding is that this >4GB is only relevant >for >64-bit SoCs, where we mandate FIT images, don't we? It's also relevant with A80, theortically. And Allwinner is continously producing Cortex-A7 SoCs, which might use this design. > >> I guess we'd need to find another way (put some information in an >> SRAM somewhere?) to try to do that for all variants. > >Oh, so your aim at getting rid of the call to the memory probing >function at all? I was just assuming that we change it for the purpose >of this 3GB support, which would be 64-bit only? > >Cheers, >Andre
于 2018年4月3日 GMT+08:00 下午7:34:55, Maxime Ripard <maxime.ripard@bootlin.com> 写到: >On Tue, Apr 03, 2018 at 11:13:17AM +0100, Andre Przywara wrote: >> >>>> For hacking it, see my implementation in v1, which assumes the >> >>>> only size supported bigger than 2GiB is 3GiB (which is >> >>>> acceptable on sunxi, but might not work on other platforms). >> >>>> >> >>>> As Andre said, that function has another big problem -- it >detects >> >>>> memory with writing to it. This is risky. >> >>> >> >>> How is it risky when it's done by the SPL? >> >> >> >> Originally that was my confusion as well: It's not the SPL calling >that >> >> function. The DRAM controller init function in there knows very >> >> precisely how much DRAM we have, but we don't communicate this to >U-Boot >> >> proper. So U-Boot *proper* goes ahead and probes the DRAM. This >means it >> >> could step into secure memory, for instance. On sunxi64 we have >the ATF >> >> running between SPL and U-Boot, also all kind of secure payloads >could >> >> already have been registered. >> >> So I wonder if it would be easier to somehow pass on this *one* >word of >> >> information between SPL and U-Boot proper to avoid calling this >function >> >> altogether? >> > >> > That would definitely make sense yes. >> >> So since the SPL loads the DT anyway (from the FIT image) and puts it >at >> the end of the U-Boot (proper) binary, wouldn't it be the easiest to >> just patch the actual DRAM size in there? >> IIRC we don't have any FDT write code in the SPL at the moment, and >> pulling it in would probably push it over the edge again, but: > >That assumes that you are loading a FIT image, which might or might >not be the case, and on most arm32 chips, most likely won't. > >I guess we'd need to find another way (put some information in an >SRAM somewhere?) to try to do that for all variants. Extend the SPL header again? If we found SPL v3+, use the DRAM size encoded and bypass ram_get_size, otherwise fallback to ram_get_size? (Although it will lead to some days of mess on sunxi-tools, this is a reasonable choice.) > >Maxime
Hi Icenowy, On 03/04/18 12:51, Icenowy Zheng wrote: > > > 于 2018年4月3日 GMT+08:00 下午7:34:55, Maxime Ripard <maxime.ripard@bootlin.com> 写到: >> On Tue, Apr 03, 2018 at 11:13:17AM +0100, Andre Przywara wrote: >>>>>>> For hacking it, see my implementation in v1, which assumes the >>>>>>> only size supported bigger than 2GiB is 3GiB (which is >>>>>>> acceptable on sunxi, but might not work on other platforms). >>>>>>> >>>>>>> As Andre said, that function has another big problem -- it >> detects >>>>>>> memory with writing to it. This is risky. >>>>>> >>>>>> How is it risky when it's done by the SPL? >>>>> >>>>> Originally that was my confusion as well: It's not the SPL calling >> that >>>>> function. The DRAM controller init function in there knows very >>>>> precisely how much DRAM we have, but we don't communicate this to >> U-Boot >>>>> proper. So U-Boot *proper* goes ahead and probes the DRAM. This >> means it >>>>> could step into secure memory, for instance. On sunxi64 we have >> the ATF >>>>> running between SPL and U-Boot, also all kind of secure payloads >> could >>>>> already have been registered. >>>>> So I wonder if it would be easier to somehow pass on this *one* >> word of >>>>> information between SPL and U-Boot proper to avoid calling this >> function >>>>> altogether? >>>> >>>> That would definitely make sense yes. >>> >>> So since the SPL loads the DT anyway (from the FIT image) and puts it >> at >>> the end of the U-Boot (proper) binary, wouldn't it be the easiest to >>> just patch the actual DRAM size in there? >>> IIRC we don't have any FDT write code in the SPL at the moment, and >>> pulling it in would probably push it over the edge again, but: >> >> That assumes that you are loading a FIT image, which might or might >> not be the case, and on most arm32 chips, most likely won't. >> >> I guess we'd need to find another way (put some information in an >> SRAM somewhere?) to try to do that for all variants. > > Extend the SPL header again? If we found SPL v3+, use > the DRAM size encoded and bypass ram_get_size, > otherwise fallback to ram_get_size? Yes, that would be a possibility as well. Though I believe at the moment we don't access the SPL header from U-Boot proper, do we? Not a real show-stopper, but we probably need to document that the SPL header would need to stay around. But if we have a fallback anyway ... > (Although it will lead to some days of mess on sunxi-tools, > this is a reasonable choice.) True, but actually I wonder why we have SPL_MAX_VERSION in there in the first place. Can't we just postulate that every new SPL version stays backwards compatible? So if sunxi-tools can deal with v2, a v3 SPL would still be fine, you would just loose the v3 features (if at all)? We can just put a warning in there, to ask users to upgrade. That would have worked already with the v1/v2 transition, I believe. Probably worth a separate discussion with some sunxi-tools stakeholders. Cheers, Andre.
On Tue, 3 Apr 2018 13:43:43 +0100 Andre Przywara <andre.przywara@arm.com> wrote: > Hi Icenowy, > > On 03/04/18 12:51, Icenowy Zheng wrote: > > > > > > 于 2018年4月3日 GMT+08:00 下午7:34:55, Maxime Ripard <maxime.ripard@bootlin.com> 写到: > >> On Tue, Apr 03, 2018 at 11:13:17AM +0100, Andre Przywara wrote: > >>>>>>> For hacking it, see my implementation in v1, which assumes the > >>>>>>> only size supported bigger than 2GiB is 3GiB (which is > >>>>>>> acceptable on sunxi, but might not work on other platforms). > >>>>>>> > >>>>>>> As Andre said, that function has another big problem -- it > >> detects > >>>>>>> memory with writing to it. This is risky. > >>>>>> > >>>>>> How is it risky when it's done by the SPL? > >>>>> > >>>>> Originally that was my confusion as well: It's not the SPL calling > >> that > >>>>> function. The DRAM controller init function in there knows very > >>>>> precisely how much DRAM we have, but we don't communicate this to > >> U-Boot > >>>>> proper. So U-Boot *proper* goes ahead and probes the DRAM. This > >> means it > >>>>> could step into secure memory, for instance. On sunxi64 we have > >> the ATF > >>>>> running between SPL and U-Boot, also all kind of secure payloads > >> could > >>>>> already have been registered. > >>>>> So I wonder if it would be easier to somehow pass on this *one* > >> word of > >>>>> information between SPL and U-Boot proper to avoid calling this > >> function > >>>>> altogether? > >>>> > >>>> That would definitely make sense yes. > >>> > >>> So since the SPL loads the DT anyway (from the FIT image) and puts it > >> at > >>> the end of the U-Boot (proper) binary, wouldn't it be the easiest to > >>> just patch the actual DRAM size in there? > >>> IIRC we don't have any FDT write code in the SPL at the moment, and > >>> pulling it in would probably push it over the edge again, but: > >> > >> That assumes that you are loading a FIT image, which might or might > >> not be the case, and on most arm32 chips, most likely won't. > >> > >> I guess we'd need to find another way (put some information in an > >> SRAM somewhere?) to try to do that for all variants. > > > > Extend the SPL header again? If we found SPL v3+, use > > the DRAM size encoded and bypass ram_get_size, > > otherwise fallback to ram_get_size? > > Yes, that would be a possibility as well. Though I believe at the moment > we don't access the SPL header from U-Boot proper, do we? We do. The boot device and also the boot.scr location (in the case of FEL boot) is read from the SPL header by the main U-Boot part. > Not a real show-stopper, but we probably need to document that the SPL > header would need to stay around. Maybe. > But if we have a fallback anyway ... Which fallback? Do you mean calling things like ram_get_size() from U-Boot? We should never do this because this is very wrong. The D-Cache may be already enabled, causing all kinds of weird effects. Also modifying data in DRAM (even temporarily) while our code is already running from DRAM is dangerous. > > (Although it will lead to some days of mess on sunxi-tools, > > this is a reasonable choice.) > > True, but actually I wonder why we have SPL_MAX_VERSION in there in the > first place. Can't we just postulate that every new SPL version stays > backwards compatible? So if sunxi-tools can deal with v2, a v3 SPL would > still be fine, you would just loose the v3 features (if at all)? > We can just put a warning in there, to ask users to upgrade. > That would have worked already with the v1/v2 transition, I believe. Yes, that's more or less how this was supposed to work in sunxi-tools from the very beginning. Except that we unfortunately got a bug in this code, which has been reported back in July 2017 and is still not resolved due to various reasons: https://github.com/linux-sunxi/sunxi-tools/issues/104 But hopefully it can be fixed sooner or later. > Probably worth a separate discussion with some sunxi-tools stakeholders. On the U-Boot side we can just increase the version number as long as the new header is a backwards compatible superset of the old one. In the unlikely case if we suddenly have to introduce a compatibility breaking change to the SPL header format, we can always change the SPL header signature from 'SPL' to something else.
On 03/04/18 15:13, Siarhei Siamashka wrote: Hi Siarhei, thanks for chiming in! > On Tue, 3 Apr 2018 13:43:43 +0100 > Andre Przywara <andre.przywara@arm.com> wrote: >> On 03/04/18 12:51, Icenowy Zheng wrote: .... >>>> I guess we'd need to find another way (put some information in an >>>> SRAM somewhere?) to try to do that for all variants. >>> >>> Extend the SPL header again? If we found SPL v3+, use >>> the DRAM size encoded and bypass ram_get_size, >>> otherwise fallback to ram_get_size? >> >> Yes, that would be a possibility as well. Though I believe at the moment >> we don't access the SPL header from U-Boot proper, do we? > > We do. The boot device and also the boot.scr location (in the case of > FEL boot) is read from the SPL header by the main U-Boot part. Ah, true, forgot about that. Even better then. >> Not a real show-stopper, but we probably need to document that the SPL >> header would need to stay around. > > Maybe. > >> But if we have a fallback anyway ... > > Which fallback? Do you mean calling things like ram_get_size() > from U-Boot? Yes, that was what Icenowy suggested: If SPL version > 2, we use the information from there, otherwise we fall back to the current behaviour, which is to ride through the DRAM and hope for the best. Though I am not sure this is really needed, as I don't see a strong reason to combine different versions of SPL and U-Boot proper (apart from FEL, maybe). > We should never do this because this is very wrong. I mostly agree, though it's not too bad, since we have quite a controlled environment. But if we can get rid of it: Yes, we should. > The D-Cache may be already enabled, causing all kinds of weird > effects. Also modifying data in DRAM (even temporarily) while > our code is already running from DRAM is dangerous. I don't see immediately how the D$ could get nasty here, but as I said above, we should do it. >>> (Although it will lead to some days of mess on sunxi-tools, >>> this is a reasonable choice.) >> >> True, but actually I wonder why we have SPL_MAX_VERSION in there in the >> first place. Can't we just postulate that every new SPL version stays >> backwards compatible? So if sunxi-tools can deal with v2, a v3 SPL would >> still be fine, you would just loose the v3 features (if at all)? >> We can just put a warning in there, to ask users to upgrade. >> That would have worked already with the v1/v2 transition, I believe. > > Yes, that's more or less how this was supposed to work in sunxi-tools > from the very beginning. Except that we unfortunately got a bug in > this code, which has been reported back in July 2017 and is still not > resolved due to various reasons: Well, that sounds more like a design issue to me: Defining the latest currently supported version as the maximum. > https://github.com/linux-sunxi/sunxi-tools/issues/104 > > But hopefully it can be fixed sooner or later. I think we can do it now, see below. >> Probably worth a separate discussion with some sunxi-tools stakeholders. > > On the U-Boot side we can just increase the version number as long as > the new header is a backwards compatible superset of the old one. > > In the unlikely case if we suddenly have to introduce a compatibility > breaking change to the SPL header format, we can always change the SPL > header signature from 'SPL' to something else. Or we could introduce some major/minor scheme, with a major change breaking compatibility, whereas a minor change does not. Then we split the uint8_t version into 2 bits of major and 6 bits of minor, for instance. So we just document this and bump SPL_MAX_VERSION now to 0x3f and are good for a while. Thoughts? Cheers, Andre.
diff --git a/arch/arm/include/asm/arch-sunxi/dram.h b/arch/arm/include/asm/arch-sunxi/dram.h index d08b82371d..7443fc3507 100644 --- a/arch/arm/include/asm/arch-sunxi/dram.h +++ b/arch/arm/include/asm/arch-sunxi/dram.h @@ -35,5 +35,8 @@ unsigned long long sunxi_dram_init(void); void mctl_await_completion(u32 *reg, u32 mask, u32 val); bool mctl_mem_matches(u32 offset); +#if defined(CONFIG_DRAM_CAN_RECALCULATE_SIZE) +unsigned long long sunxi_dram_recalculate_size(void); +#endif #endif /* _SUNXI_DRAM_H */ diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig index 029821c82d..b206472ead 100644 --- a/arch/arm/mach-sunxi/Kconfig +++ b/arch/arm/mach-sunxi/Kconfig @@ -6,6 +6,13 @@ config SPL_LDSCRIPT config IDENT_STRING default " Allwinner Technology" +config DRAM_CAN_RECALCULATE_SIZE + bool + help + Select this if the DRAM controller driver is capable of + re-calculating the size in main U-Boot. It's usable for + size bigger than 2GiB. + config DRAM_SUN4I bool help diff --git a/board/sunxi/board.c b/board/sunxi/board.c index 73dd41437d..96192a7ec3 100644 --- a/board/sunxi/board.c +++ b/board/sunxi/board.c @@ -256,7 +256,16 @@ int board_init(void) int dram_init(void) { +#ifndef CONFIG_DRAM_CAN_RECALCULATE_SIZE gd->ram_size = get_ram_size((long *)PHYS_SDRAM_0, PHYS_SDRAM_0_SIZE); +#else + unsigned long long real_dram_size = sunxi_dram_recalculate_size(); + + if (real_dram_size > CONFIG_SUNXI_DRAM_MAX_SIZE) + gd->ram_size = CONFIG_SUNXI_DRAM_MAX_SIZE; + else + gd->ram_size = (phys_size_t)real_dram_size; +#endif return 0; }
The get_ram_size() function in U-Boot can only deal with memory size smaller than 2GiB. To enable the support of 3GiB DRAM on newer 64-bit SoCs, an alternative way to detect DRAM size is needed. Add the possibility to use some DRAM size recalculating code in DRAM driver. Signed-off-by: Icenowy Zheng <icenowy@aosc.io> --- New patch in v2. arch/arm/include/asm/arch-sunxi/dram.h | 3 +++ arch/arm/mach-sunxi/Kconfig | 7 +++++++ board/sunxi/board.c | 8 ++++++++ 3 files changed, 18 insertions(+)