Re: [Xen-devel] [PATCH v2 28/29] ArmVirtualizationPkg/VirtFdtDxe: wire up XenBusDxe to "xen, xen" DT node

[Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>]

On 27 January 2015 at 11:57, Julien Grall <julien.grall@xxxxxxxxxx> wrote:
> Hi Ard,
>
> On 26/01/15 19:03, Ard Biesheuvel wrote:
>>  typedef struct {
>> @@ -66,6 +68,7 @@ STATIC CONST PROPERTY CompatibleProperties[] = {
>>    { PropertyTypePsci,    "arm,psci-0.2"        },
>>    { PropertyTypeFwCfg,   "qemu,fw-cfg-mmio"    },
>>    { PropertyTypeGicV3,   "arm,gic-v3"          },
>> +  { PropertyTypeXen,     "xen,xen"             },
>>    { PropertyTypeUnknown, ""                    }
>>  };
>>
>> @@ -332,6 +335,26 @@ InitializeVirtFdtDxe (
>>        }
>>        break;
>>
>> +    case PropertyTypeXen:
>> +      ASSERT (Len == 16);
>> +
>
> I would not assume that the reg property is always 16 bytes (8 bytes for
> the address and 8 bytes for the size). We may decide to change it in the
> future. That's why #address-cells and #size-cells exist in the DT.
>

Yes, you are quite correct. However, this code was originally created
as a counterpart to QEMU/mach-virt, which is known to use 64-bit
quantities for memory ranges, and adding variable address size support
to it implies that we need to start caring about how the nodes are
nested, which we currently don't. (#address-cells and #size-cells
properties are inherited by child nodes)

For Xen on arm64, #address-cells = 2 and #size-cells = 2 is the only
meaningful option anyway, but I agree that blindly assuming it is not
the most elegant approach.

> But it looks like that the other part of the code in this function
> always assume a fixed length. I guess we could live with it. I would add
> a comment explaining this restriction.
>

Indeed.

-- 
Ard.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel