| Message ID | 1423058539-26403-34-git-send-email-parth.dixit@linaro.org |
|---|---|
| State | New |
| Headers | show |
Hi Parth, On 04/02/2015 14:02, parth.dixit@linaro.org wrote: > From: Parth Dixit <parth.dixit@linaro.org> > > efi should be enabled to fetch the root pointer from uefi > > Signed-off-by: Parth Dixit <parth.dixit@linaro.org> > --- > xen/common/efi/runtime.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/xen/common/efi/runtime.c b/xen/common/efi/runtime.c > index c840e08..c8b8642 100644 > --- a/xen/common/efi/runtime.c > +++ b/xen/common/efi/runtime.c > @@ -11,7 +11,13 @@ DEFINE_XEN_GUEST_HANDLE(CHAR16); > #ifndef COMPAT > > #ifdef CONFIG_ARM /* Disabled until runtime services implemented */ This comment seems irrelevant now. > + > +#if defined(CONFIG_ARM_64) && defined(CONFIG_ACPI) #ifdef CONFIG_ACPI > +const bool_t efi_enabled = 1; > +#else > const bool_t efi_enabled = 0; > +#endif > + > #else > # include <asm/i387.h> > # include <asm/xstate.h> > Regards,
On 5 February 2015 at 11:01, Julien Grall <julien.grall@linaro.org> wrote: > Hi Parth, > > On 04/02/2015 14:02, parth.dixit@linaro.org wrote: >> >> From: Parth Dixit <parth.dixit@linaro.org> >> >> efi should be enabled to fetch the root pointer from uefi >> >> Signed-off-by: Parth Dixit <parth.dixit@linaro.org> >> --- >> xen/common/efi/runtime.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/xen/common/efi/runtime.c b/xen/common/efi/runtime.c >> index c840e08..c8b8642 100644 >> --- a/xen/common/efi/runtime.c >> +++ b/xen/common/efi/runtime.c >> @@ -11,7 +11,13 @@ DEFINE_XEN_GUEST_HANDLE(CHAR16); >> #ifndef COMPAT >> >> #ifdef CONFIG_ARM /* Disabled until runtime services implemented */ > > > This comment seems irrelevant now. i will move it further down. >> + >> +#if defined(CONFIG_ARM_64) && defined(CONFIG_ACPI) > > > #ifdef CONFIG_ACPI > >> +const bool_t efi_enabled = 1; >> +#else >> const bool_t efi_enabled = 0; >> +#endif >> + >> #else >> # include <asm/i387.h> >> # include <asm/xstate.h> >> > > Regards, > > -- > Julien Grall
(Adding Roy) Hi Jan and Roy, On 04/02/2015 22:02, parth.dixit@linaro.org wrote: > From: Parth Dixit <parth.dixit@linaro.org> > > efi should be enabled to fetch the root pointer from uefi > > Signed-off-by: Parth Dixit <parth.dixit@linaro.org> > --- > xen/common/efi/runtime.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/xen/common/efi/runtime.c b/xen/common/efi/runtime.c > index c840e08..c8b8642 100644 > --- a/xen/common/efi/runtime.c > +++ b/xen/common/efi/runtime.c > @@ -11,7 +11,13 @@ DEFINE_XEN_GUEST_HANDLE(CHAR16); > #ifndef COMPAT > > #ifdef CONFIG_ARM /* Disabled until runtime services implemented */ > + > +#if defined(CONFIG_ARM_64) && defined(CONFIG_ACPI) > +const bool_t efi_enabled = 1; > +#else > const bool_t efi_enabled = 0; > +#endif > + Thinking a bit more about this variable. The usage of the variable is unclear. Does it mean that Xen is supporting EFI runtime or Xen has been boot with EFI? The binary produced on ARM64 always is able to boot with and without EFI. So it would seems logical to me that the value of this variable may change at runtime. Regards,
Hi Ian, On 05/02/2015 20:05, Ian Campbell wrote: > On Thu, 2015-02-05 at 11:58 +0000, Jan Beulich wrote: >>>>> On 05.02.15 at 06:31, <julien.grall@linaro.org> wrote: >>>> --- a/xen/common/efi/runtime.c >>>> +++ b/xen/common/efi/runtime.c >>>> @@ -11,7 +11,13 @@ DEFINE_XEN_GUEST_HANDLE(CHAR16); >>>> #ifndef COMPAT >>>> >>>> #ifdef CONFIG_ARM /* Disabled until runtime services implemented */ >>> >>> This comment seems irrelevant now. >>> >>>> + >>>> +#if defined(CONFIG_ARM_64) && defined(CONFIG_ACPI) >>> >>> #ifdef CONFIG_ACPI >> >> This is common code, and I can't see ACPI and EFI being always in the >> same supported state (or else we could drop one of the two). > > EFI without ACPI is certainly a possibility on ARM64. We would need to defined a new protocol in order to boot ACPI without EFI. Currently the ACPI fetch the rsdp pointer in 2 differents way depending of efi_enabled: * efi_enabled == 1 => Use EFI to get the pointer * efi_enabled == 0 => Use the x86 legacy mode On ARM64, we have to use the first one. Maybe we should refactor acpi_os_get_root_pointer? Regards,
Hi Jan, On 11/02/2015 18:31, Jan Beulich wrote: >>>> On 11.02.15 at 10:57, <julien.grall@linaro.org> wrote: >> Hi Ian, >> >> On 05/02/2015 20:05, Ian Campbell wrote: >>> On Thu, 2015-02-05 at 11:58 +0000, Jan Beulich wrote: >>>>>>> On 05.02.15 at 06:31, <julien.grall@linaro.org> wrote: >>>>>> --- a/xen/common/efi/runtime.c >>>>>> +++ b/xen/common/efi/runtime.c >>>>>> @@ -11,7 +11,13 @@ DEFINE_XEN_GUEST_HANDLE(CHAR16); >>>>>> #ifndef COMPAT >>>>>> >>>>>> #ifdef CONFIG_ARM /* Disabled until runtime services implemented */ >>>>> >>>>> This comment seems irrelevant now. >>>>> >>>>>> + >>>>>> +#if defined(CONFIG_ARM_64) && defined(CONFIG_ACPI) >>>>> >>>>> #ifdef CONFIG_ACPI >>>> >>>> This is common code, and I can't see ACPI and EFI being always in the >>>> same supported state (or else we could drop one of the two). >>> >>> EFI without ACPI is certainly a possibility on ARM64. >> >> We would need to defined a new protocol in order to boot ACPI without EFI. >> >> Currently the ACPI fetch the rsdp pointer in 2 differents way depending >> of efi_enabled: >> * efi_enabled == 1 => Use EFI to get the pointer >> * efi_enabled == 0 => Use the x86 legacy mode >> >> On ARM64, we have to use the first one. > > How that when not booting from EFI? Surely you can't use x86 > legacy mode, but if there is the possibility of ACPI without EFI > (the opposite of what Ian indicated would be a possibility), then > there ought to be another method to find RSDP on ARM too. I should have been more clear on my previous mail. I didn't try to justify this patch (I think it's wrong too), but I wanted to expose our current use-case for ACPI. We would, obviously, have to implement it when a new way to get ACPI is coming up. Currently, ACPI can only be retrieved via EFI on ARM64. So if efi_enabled is not set, we should bail out rather than trying to use the legacy mode. It would help to support correctly ARM64 platform with only DT support (and not EFI). Regards,
diff --git a/xen/common/efi/runtime.c b/xen/common/efi/runtime.c index c840e08..c8b8642 100644 --- a/xen/common/efi/runtime.c +++ b/xen/common/efi/runtime.c @@ -11,7 +11,13 @@ DEFINE_XEN_GUEST_HANDLE(CHAR16); #ifndef COMPAT #ifdef CONFIG_ARM /* Disabled until runtime services implemented */ + +#if defined(CONFIG_ARM_64) && defined(CONFIG_ACPI) +const bool_t efi_enabled = 1; +#else const bool_t efi_enabled = 0; +#endif + #else # include <asm/i387.h> # include <asm/xstate.h>