On 7 February 2017 at 19:07, Jeffrey Hugo <jhugo@xxxxxxxxxxxxxx> wrote: > On 2/7/2017 12:01 PM, Mark Rutland wrote: >> >> On Tue, Feb 07, 2017 at 11:54:55AM -0700, Jeffrey Hugo wrote: >>> >>> On 2/7/2017 11:12 AM, Ard Biesheuvel wrote: >>>> >>>> On 7 February 2017 at 17:59, Jeffrey Hugo <jhugo@xxxxxxxxxxxxxx> wrote: >>>>> >>>>> From: Sameer Goel <sgoel@xxxxxxxxxxxxxx> >>>>> >>>>> In cases where a device tree is not provided (ie ACPI based system), an >>>>> empty fdt is generated by efistub. Sets the address and size cell >>>>> values >>>>> in a generated fdt to support 64 bit addressing. >>>>> >>>>> This enables kexec/kdump on Qualcomm Technologies QDF24XX platforms as >>>>> those >>>>> utilities will read the address/size values from the fdt, and such >>>>> values >>>>> may exceed the range provided by the 32 bit default. >>>>> >>>> >>>> As far as I know, those properties are explicitly associated with the >>>> 'reg' properties of subordinate nodes. So which nodes are we talking >>>> about here? Are we producing an incorrect DT by not setting these? Or >>>> is this simply a convenience to work around bugs in the tooling? >>> >>> >>> I think we are producing an incorrect DT, in some instances. >>> >>> So we are starting from the same baseline, this is specific to ACPI >>> systems, as an ACPI system won't have a DT from the bootloader. DT >>> based systems will already have a DT from the bootloader which is >>> assumed to be correct. On ACPI systems without a DT, efistub >>> generates a default one. >>> >>> That default is assumed to be for a 32-bit system. The cell width >>> defaults to 1, which is 4 bytes. You cannot represent a 64-bit >>> value in that instance. >>> >>> What happens is that kexec inserts properties into the fdt which >>> contain the start address and size on the crash kernel. On our >>> system, the start address is a 64-bit value, and while its not the >>> case today, I see no reason why size could not also be a 64-bit >>> value. However the values that are inserted into the fdt are >>> governed by the address and size cell values already present in the >>> fdt. >>> >>> Kexec attempts to insert these values in the fdt. The fdt only >>> accepts 32-bit values, so it truncates what is put in. Then later >>> kexec/kdump read the values from the fdt, and get garbage. >> >> >> I take it this is specific to the kdump properties? >> >> I can't immediately see what would matter for the !kdump case. >> properties inserted under /chosen are not truncated? > > > The kexec/kdump properties are added under /chosen, therefore yes, > properties added under /chosen are truncated, per our observations. > You still haven't told use the name of those properties :-) If they are not 'reg' properties, why do #address-cells & #size-cells matter at all? Is it the dtc tool that does this internally? >> >>> By changing the defaults to 2 (the proposed change), 64-bit values >>> can be inserted into the fdt, so the values we put in don't get >>> truncated, and thus kexec/kdump read the correct thing when they >>> need the values. >>> >>> I don't see how the tools could be fixed - fdt is truncating the >>> values, and the generated fdt is already "static" at the point the >>> tools run. We haven't had luck changing the cell size at the point >>> the tools run. Additionally, this seems to be an issue for >>> everything using the fdt - pushing the problem to every tool instead >>> of fixing once at the top seems like playing a game of whack-a-mole. >>> >>> Does that clarify that issue for you? Obviously the commit text >>> needs some work, but I'd like to get on the same page first. >> >> >> If you could update the commit message to explicitly mention the >> properties being inserted for which this matters, this generally sounds >> fine to me. >> >> Please Cc me on v2. >> Likewise -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
