Re: [Xen-devel] [PATCH 08/13] Xen: EFI: Parse DT parameters for Xen specific UEFI

[Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>]

On 17 November 2015 at 12:37, Mark Rutland <mark.rutland@xxxxxxx> wrote:
> On Tue, Nov 17, 2015 at 12:25:58PM +0100, Ard Biesheuvel wrote:
>> On 17 November 2015 at 10:57,  <shannon.zhao@xxxxxxxxxx> wrote:
>> > From: Shannon Zhao <shannon.zhao@xxxxxxxxxx>
>> >
>> > Add a new function to parse DT parameters for Xen specific UEFI just
>> > like the way for normal UEFI. Then it could reuse the existing codes.
>> >
>> > Signed-off-by: Shannon Zhao <shannon.zhao@xxxxxxxxxx>
>> > ---
>> >  drivers/firmware/efi/efi.c | 67 
>> > ++++++++++++++++++++++++++++++++++++++++++----
>> >  1 file changed, 62 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
>> > index d6144e3..629bd06 100644
>> > --- a/drivers/firmware/efi/efi.c
>> > +++ b/drivers/firmware/efi/efi.c
>> > @@ -24,6 +24,7 @@
>> >  #include <linux/of_fdt.h>
>> >  #include <linux/io.h>
>> >  #include <linux/platform_device.h>
>> > +#include <xen/xen.h>
>> >
>> >  struct efi __read_mostly efi = {
>> >         .mps        = EFI_INVALID_TABLE_ADDR,
>> > @@ -488,12 +489,60 @@ static __initdata struct {
>> >         UEFI_PARAM("MemMap Desc. Version", "linux,uefi-mmap-desc-ver", 
>> > desc_ver)
>> >  };
>> >
>> > +static __initdata struct {
>> > +       const char name[32];
>> > +       const char propname[32];
>> > +       int offset;
>> > +       int size;
>> > +} xen_dt_params[] = {
>> > +       UEFI_PARAM("System Table", "xen,uefi-system-table", system_table),
>> > +       UEFI_PARAM("MemMap Address", "xen,uefi-mmap-start", mmap),
>> > +       UEFI_PARAM("MemMap Size", "xen,uefi-mmap-size", mmap_size),
>> > +       UEFI_PARAM("MemMap Desc. Size", "xen,uefi-mmap-desc-size", 
>> > desc_size),
>> > +       UEFI_PARAM("MemMap Desc. Version", "xen,uefi-mmap-desc-ver", 
>> > desc_ver)
>> > +};
>> > +
>>
>> We discussed (and agreed afair) that dropping the "linux," prefix from
>> the DT properties was an acceptable change. If we do that, and reuse
>> the same names in the xen version (i.e., drop the "xen," prefix), we
>> could make this change a *lot* simpler.
>
> Regardless of if we drop the "linux," prefix from the existing strings,
> I think we need the "xen," prefix here.
>
> The xen EFI interface comes with additional caveats, and we need to
> treat it separately.
>
> Unless you're suggesting that /hypervisor/uefi-* is handled differently
> to /chosen/uefi-*?
>
> I think I'd still prefer the "xen," prefix regardless.
>

Well, we should still be able to reuse more of the existing code
rather than duplicating a bunch of code in fdt_find_xen_uefi_params()

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