| Message ID | 1470438282-4226-13-git-send-email-daniel.kiper@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 06.08.16 at 01:04, <daniel.kiper@oracle.com> wrote: Apart from the question of this probably better getting merged with the previous patch ... > --- a/xen/common/efi/boot.c > +++ b/xen/common/efi/boot.c > @@ -936,6 +936,10 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) > > __set_bit(EFI_BOOT, &efi.flags); > > +#ifndef CONFIG_ARM /* Disabled until runtime services implemented. */ > + __set_bit(EFI_RS, &efi.flags); > +#endif ... this needs to be paralleled by a __clear_bit() when "efi=no-rs" was given (and then efi_rs_enable) should go away. Oh, looks like that's the next patch, but I'd then again question the split. > --- a/xen/include/xen/efi.h > +++ b/xen/include/xen/efi.h > @@ -12,6 +12,7 @@ > struct efi { > unsigned long flags; /* Bit fields representing available EFI features/properties */ > #define EFI_BOOT 0 /* Were we booted from EFI? */ > +#define EFI_RS 2 /* Can we use runtime services? */ Why is this not the sequentially next number? Jan
On Wed, Aug 17, 2016 at 10:12:32AM -0600, Jan Beulich wrote: > >>> On 06.08.16 at 01:04, <daniel.kiper@oracle.com> wrote: > > Apart from the question of this probably better getting merged with > the previous patch ... > > > --- a/xen/common/efi/boot.c > > +++ b/xen/common/efi/boot.c > > @@ -936,6 +936,10 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) > > > > __set_bit(EFI_BOOT, &efi.flags); > > > > +#ifndef CONFIG_ARM /* Disabled until runtime services implemented. */ > > + __set_bit(EFI_RS, &efi.flags); > > +#endif > > ... this needs to be paralleled by a __clear_bit() when "efi=no-rs" > was given (and then efi_rs_enable) should go away. Oh, looks > like that's the next patch, but I'd then again question the split. Do you suggest that patches 11-13 should be merged into one thing? If it is OK for you I can do that. > > --- a/xen/include/xen/efi.h > > +++ b/xen/include/xen/efi.h > > @@ -12,6 +12,7 @@ > > struct efi { > > unsigned long flags; /* Bit fields representing available EFI features/properties */ > > #define EFI_BOOT 0 /* Were we booted from EFI? */ > > +#define EFI_RS 2 /* Can we use runtime services? */ > > Why is this not the sequentially next number? I wish that EFI_LOADER (added in patch 16) has number 1 (next to EFI_BOOT). Daniel
>>> On 18.08.16 at 12:30, <daniel.kiper@oracle.com> wrote: > On Wed, Aug 17, 2016 at 10:12:32AM -0600, Jan Beulich wrote: >> >>> On 06.08.16 at 01:04, <daniel.kiper@oracle.com> wrote: >> >> Apart from the question of this probably better getting merged with >> the previous patch ... >> >> > --- a/xen/common/efi/boot.c >> > +++ b/xen/common/efi/boot.c >> > @@ -936,6 +936,10 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE > *SystemTable) >> > >> > __set_bit(EFI_BOOT, &efi.flags); >> > >> > +#ifndef CONFIG_ARM /* Disabled until runtime services implemented. */ >> > + __set_bit(EFI_RS, &efi.flags); >> > +#endif >> >> ... this needs to be paralleled by a __clear_bit() when "efi=no-rs" >> was given (and then efi_rs_enable) should go away. Oh, looks >> like that's the next patch, but I'd then again question the split. > > Do you suggest that patches 11-13 should be merged into one thing? > If it is OK for you I can do that. > >> > --- a/xen/include/xen/efi.h >> > +++ b/xen/include/xen/efi.h >> > @@ -12,6 +12,7 @@ >> > struct efi { >> > unsigned long flags; /* Bit fields representing available EFI > features/properties */ >> > #define EFI_BOOT 0 /* Were we booted from EFI? */ >> > +#define EFI_RS 2 /* Can we use runtime services? */ >> >> Why is this not the sequentially next number? > > I wish that EFI_LOADER (added in patch 16) has number 1 (next to EFI_BOOT). That'll then too end up more natural by merging the patches. Jan
On Thu, Aug 18, 2016 at 05:18:01AM -0600, Jan Beulich wrote: > >>> On 18.08.16 at 12:30, <daniel.kiper@oracle.com> wrote: > > On Wed, Aug 17, 2016 at 10:12:32AM -0600, Jan Beulich wrote: > >> >>> On 06.08.16 at 01:04, <daniel.kiper@oracle.com> wrote: > >> > >> Apart from the question of this probably better getting merged with > >> the previous patch ... > >> > >> > --- a/xen/common/efi/boot.c > >> > +++ b/xen/common/efi/boot.c > >> > @@ -936,6 +936,10 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE > > *SystemTable) > >> > > >> > __set_bit(EFI_BOOT, &efi.flags); > >> > > >> > +#ifndef CONFIG_ARM /* Disabled until runtime services implemented. */ > >> > + __set_bit(EFI_RS, &efi.flags); > >> > +#endif > >> > >> ... this needs to be paralleled by a __clear_bit() when "efi=no-rs" > >> was given (and then efi_rs_enable) should go away. Oh, looks > >> like that's the next patch, but I'd then again question the split. > > > > Do you suggest that patches 11-13 should be merged into one thing? > > If it is OK for you I can do that. > > > >> > --- a/xen/include/xen/efi.h > >> > +++ b/xen/include/xen/efi.h > >> > @@ -12,6 +12,7 @@ > >> > struct efi { > >> > unsigned long flags; /* Bit fields representing available EFI > > features/properties */ > >> > #define EFI_BOOT 0 /* Were we booted from EFI? */ > >> > +#define EFI_RS 2 /* Can we use runtime services? */ > >> > >> Why is this not the sequentially next number? > > > > I wish that EFI_LOADER (added in patch 16) has number 1 (next to EFI_BOOT). > > That'll then too end up more natural by merging the patches. Potentially we can do that but patch 14 and 15 use efi_enabled(). So, we should merge patches 11-13 if you wish and leave 14-16 as is. Daniel
diff --git a/xen/arch/x86/domain_page.c b/xen/arch/x86/domain_page.c index 71ade05..7541b91 100644 --- a/xen/arch/x86/domain_page.c +++ b/xen/arch/x86/domain_page.c @@ -36,7 +36,7 @@ static inline struct vcpu *mapcache_current_vcpu(void) * domain's page tables but current may point at another domain's VCPU. * Return NULL as though current is not properly set up yet. */ - if ( efi_enabled(EFI_BOOT) && efi_rs_using_pgtables() ) + if ( efi_enabled(EFI_RS) && efi_rs_using_pgtables() ) return NULL; /* diff --git a/xen/arch/x86/shutdown.c b/xen/arch/x86/shutdown.c index 7ce3761..b429fd0 100644 --- a/xen/arch/x86/shutdown.c +++ b/xen/arch/x86/shutdown.c @@ -119,7 +119,7 @@ void machine_halt(void) static void default_reboot_type(void) { if ( reboot_type == BOOT_INVALID ) - reboot_type = efi_enabled(EFI_BOOT) ? BOOT_EFI + reboot_type = efi_enabled(EFI_RS) ? BOOT_EFI : acpi_disabled ? BOOT_KBD : BOOT_ACPI; } diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c index b2ecc8e..8d94530 100644 --- a/xen/arch/x86/time.c +++ b/xen/arch/x86/time.c @@ -686,7 +686,7 @@ static unsigned long get_cmos_time(void) static bool_t __read_mostly cmos_rtc_probe; boolean_param("cmos-rtc-probe", cmos_rtc_probe); - if ( efi_enabled(EFI_BOOT) ) + if ( efi_enabled(EFI_RS) ) { res = efi_get_time(); if ( res ) diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c index edd0434..dd6b0a8 100644 --- a/xen/common/efi/boot.c +++ b/xen/common/efi/boot.c @@ -936,6 +936,10 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) __set_bit(EFI_BOOT, &efi.flags); +#ifndef CONFIG_ARM /* Disabled until runtime services implemented. */ + __set_bit(EFI_RS, &efi.flags); +#endif + efi_init(ImageHandle, SystemTable); use_cfg_file = efi_arch_use_config_file(SystemTable); diff --git a/xen/include/xen/efi.h b/xen/include/xen/efi.h index be18e4d..ba14472 100644 --- a/xen/include/xen/efi.h +++ b/xen/include/xen/efi.h @@ -12,6 +12,7 @@ struct efi { unsigned long flags; /* Bit fields representing available EFI features/properties */ #define EFI_BOOT 0 /* Were we booted from EFI? */ +#define EFI_RS 2 /* Can we use runtime services? */ unsigned long mps; /* MPS table */ unsigned long acpi; /* ACPI table (IA64 ext 0.71) */ unsigned long acpi20; /* ACPI table (ACPI 2.0) */
Suggested-by: Jan Beulich <jbeulich@suse.com> Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com> --- xen/arch/x86/domain_page.c | 2 +- xen/arch/x86/shutdown.c | 2 +- xen/arch/x86/time.c | 2 +- xen/common/efi/boot.c | 4 ++++ xen/include/xen/efi.h | 1 + 5 files changed, 8 insertions(+), 3 deletions(-)