[PATCH 4.12 26/84] x86/xen/efi: Initialize only the EFI struct members used by Xen

[PATCH 4.12 26/84] x86/xen/efi: Initialize only the EFI struct members used by Xen

[Greg Kroah-Hartman]

4.12-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Daniel Kiper <daniel.kiper@oracle.com>

commit 6c64447ec58b0bac612732303f7ab04562124587 upstream.

The current approach, which is the wholesale efi struct initialization from
a 'efi_xen' local template is not robust. Usually if new member is defined
then it is properly initialized in drivers/firmware/efi/efi.c, but not in
arch/x86/xen/efi.c.

The effect is that the Xen initialization clears any fields the generic code
might have set and the Xen code does not know about yet.

I saw this happen a few times, so let's initialize only the EFI struct members
used by Xen and maintain no local duplicate, to avoid such issues in the future.

Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: andrew.cooper3@citrix.com
Cc: jgross@suse.com
Cc: linux-efi@vger.kernel.org
Cc: matt@codeblueprint.co.uk
Cc: xen-devel@lists.xenproject.org
Link: http://lkml.kernel.org/r/1498128697-12943-3-git-send-email-daniel.kiper@oracle.com
[ Clarified the changelog. ]
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 arch/x86/xen/efi.c |   45 ++++++++++++---------------------------------
 1 file changed, 12 insertions(+), 33 deletions(-)

--- a/arch/x86/xen/efi.c
+++ b/arch/x86/xen/efi.c
@@ -54,38 +54,6 @@ static efi_system_table_t efi_systab_xen
 	.tables		= EFI_INVALID_TABLE_ADDR  /* Initialized later. */
 };
 
-static const struct efi efi_xen __initconst = {
-	.systab                   = NULL, /* Initialized later. */
-	.runtime_version	  = 0,    /* Initialized later. */
-	.mps                      = EFI_INVALID_TABLE_ADDR,
-	.acpi                     = EFI_INVALID_TABLE_ADDR,
-	.acpi20                   = EFI_INVALID_TABLE_ADDR,
-	.smbios                   = EFI_INVALID_TABLE_ADDR,
-	.smbios3                  = EFI_INVALID_TABLE_ADDR,
-	.sal_systab               = EFI_INVALID_TABLE_ADDR,
-	.boot_info                = EFI_INVALID_TABLE_ADDR,
-	.hcdp                     = EFI_INVALID_TABLE_ADDR,
-	.uga                      = EFI_INVALID_TABLE_ADDR,
-	.uv_systab                = EFI_INVALID_TABLE_ADDR,
-	.fw_vendor                = EFI_INVALID_TABLE_ADDR,
-	.runtime                  = EFI_INVALID_TABLE_ADDR,
-	.config_table             = EFI_INVALID_TABLE_ADDR,
-	.get_time                 = xen_efi_get_time,
-	.set_time                 = xen_efi_set_time,
-	.get_wakeup_time          = xen_efi_get_wakeup_time,
-	.set_wakeup_time          = xen_efi_set_wakeup_time,
-	.get_variable             = xen_efi_get_variable,
-	.get_next_variable        = xen_efi_get_next_variable,
-	.set_variable             = xen_efi_set_variable,
-	.query_variable_info      = xen_efi_query_variable_info,
-	.update_capsule           = xen_efi_update_capsule,
-	.query_capsule_caps       = xen_efi_query_capsule_caps,
-	.get_next_high_mono_count = xen_efi_get_next_high_mono_count,
-	.reset_system             = xen_efi_reset_system,
-	.set_virtual_address_map  = NULL, /* Not used under Xen. */
-	.flags			  = 0     /* Initialized later. */
-};
-
 static efi_system_table_t __init *xen_efi_probe(void)
 {
 	struct xen_platform_op op = {
@@ -102,7 +70,18 @@ static efi_system_table_t __init *xen_ef
 
 	/* Here we know that Xen runs on EFI platform. */
 
-	efi = efi_xen;
+	efi.get_time                 = xen_efi_get_time;
+	efi.set_time                 = xen_efi_set_time;
+	efi.get_wakeup_time          = xen_efi_get_wakeup_time;
+	efi.set_wakeup_time          = xen_efi_set_wakeup_time;
+	efi.get_variable             = xen_efi_get_variable;
+	efi.get_next_variable        = xen_efi_get_next_variable;
+	efi.set_variable             = xen_efi_set_variable;
+	efi.query_variable_info      = xen_efi_query_variable_info;
+	efi.update_capsule           = xen_efi_update_capsule;
+	efi.query_capsule_caps       = xen_efi_query_capsule_caps;
+	efi.get_next_high_mono_count = xen_efi_get_next_high_mono_count;
+	efi.reset_system             = xen_efi_reset_system;
 
 	efi_systab_xen.tables = info->cfg.addr;
 	efi_systab_xen.nr_tables = info->cfg.nent;



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

Re: [PATCH 4.12 26/84] x86/xen/efi: Initialize only the EFI struct members used by Xen

[Daniel Kiper]

Hey Greg,

On Wed, Jul 19, 2017 at 11:43:32AM +0200, Greg Kroah-Hartman wrote:
> 4.12-stable review patch.  If anyone has any objections, please let me know.

Why did you skip this patch for 4.11? IMO it should be applied there too.

Daniel

Re: [PATCH 4.12 26/84] x86/xen/efi: Initialize only the EFI struct members used by Xen

[Greg Kroah-Hartman]

On Wed, Jul 19, 2017 at 12:37:47PM +0200, Daniel Kiper wrote:
> Hey Greg,
> 
> On Wed, Jul 19, 2017 at 11:43:32AM +0200, Greg Kroah-Hartman wrote:
> > 4.12-stable review patch.  If anyone has any objections, please let me know.
> 
> Why did you skip this patch for 4.11? IMO it should be applied there too.

Are you sure it actually applied?  (hint, it did not...)

If you want it in 4.11, or older kernels, please provide a working
backport.

thanks,

greg k-h

Re: [PATCH 4.12 26/84] x86/xen/efi: Initialize only the EFI struct members used by Xen

[Greg Kroah-Hartman]

On Wed, Jul 19, 2017 at 01:12:14PM +0200, Greg Kroah-Hartman wrote:
> On Wed, Jul 19, 2017 at 12:37:47PM +0200, Daniel Kiper wrote:
> > Hey Greg,
> > 
> > On Wed, Jul 19, 2017 at 11:43:32AM +0200, Greg Kroah-Hartman wrote:
> > > 4.12-stable review patch.  If anyone has any objections, please let me know.
> > 
> > Why did you skip this patch for 4.11? IMO it should be applied there too.
> 
> Are you sure it actually applied?  (hint, it did not...)
> 
> If you want it in 4.11, or older kernels, please provide a working
> backport.

And, in the future, if you want it to be applied to older kernels, or be
notified if it can not be, please add a kernel version number in the
stable marking:
	Cc: stable@vger.kernel.org # 4.0+
or use the Fixes: tag:
	Fixes: SHASHAHSA ("short description")
which I pick up on and let you know if the patch does not actually apply
back to the kernel that the fixes: tag was in.

hope this helps,

greg k-h

Re: [PATCH 4.12 26/84] x86/xen/efi: Initialize only the EFI struct members used by Xen

[Daniel Kiper]

On Wed, Jul 19, 2017 at 01:19:58PM +0200, Greg Kroah-Hartman wrote:
> On Wed, Jul 19, 2017 at 01:12:14PM +0200, Greg Kroah-Hartman wrote:
> > On Wed, Jul 19, 2017 at 12:37:47PM +0200, Daniel Kiper wrote:
> > > Hey Greg,
> > >
> > > On Wed, Jul 19, 2017 at 11:43:32AM +0200, Greg Kroah-Hartman wrote:
> > > > 4.12-stable review patch.  If anyone has any objections, please let me know.
> > >
> > > Why did you skip this patch for 4.11? IMO it should be applied there too.
> >
> > Are you sure it actually applied?  (hint, it did not...)
> >
> > If you want it in 4.11, or older kernels, please provide a working
> > backport.
>
> And, in the future, if you want it to be applied to older kernels, or be
> notified if it can not be, please add a kernel version number in the
> stable marking:
> 	Cc: stable@vger.kernel.org # 4.0+
> or use the Fixes: tag:
> 	Fixes: SHASHAHSA ("short description")
> which I pick up on and let you know if the patch does not actually apply
> back to the kernel that the fixes: tag was in.
>
> hope this helps,

Sure thing! Thanks a lot!

Daniel

Re: [PATCH 4.12 26/84] x86/xen/efi: Initialize only the EFI struct members used by Xen

[Daniel Kiper]

On Wed, Jul 19, 2017 at 01:12:14PM +0200, Greg Kroah-Hartman wrote:
> On Wed, Jul 19, 2017 at 12:37:47PM +0200, Daniel Kiper wrote:
> > Hey Greg,
> >
> > On Wed, Jul 19, 2017 at 11:43:32AM +0200, Greg Kroah-Hartman wrote:
> > > 4.12-stable review patch.  If anyone has any objections, please let me know.
> >
> > Why did you skip this patch for 4.11? IMO it should be applied there too.
>
> Are you sure it actually applied?  (hint, it did not...)
>
> If you want it in 4.11, or older kernels, please provide a working
> backport.

OK, if it did not apply then probably there were some changes in the code
here and there. Though, IIRC, fix itself is perfectly valid for 4.11.
So, I will post updated patch for it.

Thanks,

Daniel

Re: [PATCH 4.12 26/84] x86/xen/efi: Initialize only the EFI struct members used by Xen

[Ingo Molnar]

* Daniel Kiper <daniel.kiper@oracle.com> wrote:

> Hey Greg,
> 
> On Wed, Jul 19, 2017 at 11:43:32AM +0200, Greg Kroah-Hartman wrote:
> > 4.12-stable review patch.  If anyone has any objections, please let me know.
> 
> Why did you skip this patch for 4.11? IMO it should be applied there too.

The thing is, this patch should probaly not even be in v4.12, as it should only 
make any difference if there's a separate _bug_ in the kernel.

This patch makes things more robust going forward, but I question that it needs to 
be in -stable.

Thanks,

	Ingo

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

Re: [PATCH 4.12 26/84] x86/xen/efi: Initialize only the EFI struct members used by Xen

[Greg Kroah-Hartman]

On Thu, Jul 20, 2017 at 10:39:10AM +0200, Ingo Molnar wrote:
> 
> * Daniel Kiper <daniel.kiper@oracle.com> wrote:
> 
> > Hey Greg,
> > 
> > On Wed, Jul 19, 2017 at 11:43:32AM +0200, Greg Kroah-Hartman wrote:
> > > 4.12-stable review patch.  If anyone has any objections, please let me know.
> > 
> > Why did you skip this patch for 4.11? IMO it should be applied there too.
> 
> The thing is, this patch should probaly not even be in v4.12, as it should only 
> make any difference if there's a separate _bug_ in the kernel.
> 
> This patch makes things more robust going forward, but I question that it needs to 
> be in -stable.

Yeah, good point, I'm going to go drop it entirely from the 4.12-stable
tree as it obviously isn't stable material, sorry for not catching that
before.

thanks,

greg k-h

Re: [PATCH 4.12 26/84] x86/xen/efi: Initialize only the EFI struct members used by Xen

[Ingo Molnar]

* Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org> wrote:

> On Thu, Jul 20, 2017 at 10:39:10AM +0200, Ingo Molnar wrote:
> > 
> > * Daniel Kiper <daniel.kiper-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> wrote:
> > 
> > > Hey Greg,
> > > 
> > > On Wed, Jul 19, 2017 at 11:43:32AM +0200, Greg Kroah-Hartman wrote:
> > > > 4.12-stable review patch.  If anyone has any objections, please let me know.
> > > 
> > > Why did you skip this patch for 4.11? IMO it should be applied there too.
> > 
> > The thing is, this patch should probaly not even be in v4.12, as it should only 
> > make any difference if there's a separate _bug_ in the kernel.
> > 
> > This patch makes things more robust going forward, but I question that it needs to 
> > be in -stable.
> 
> Yeah, good point, I'm going to go drop it entirely from the 4.12-stable
> tree as it obviously isn't stable material, sorry for not catching that
> before.

I should have caught the tag as well when applying the upstream patch to begin 
with.

Thanks!

	Ingo

Re: [PATCH 4.12 26/84] x86/xen/efi: Initialize only the EFI struct members used by Xen

[Daniel Kiper]

On Thu, Jul 20, 2017 at 11:16:39AM +0200, Greg Kroah-Hartman wrote:
> On Thu, Jul 20, 2017 at 10:39:10AM +0200, Ingo Molnar wrote:
> >
> > * Daniel Kiper <daniel.kiper-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> wrote:
> >
> > > Hey Greg,
> > >
> > > On Wed, Jul 19, 2017 at 11:43:32AM +0200, Greg Kroah-Hartman wrote:
> > > > 4.12-stable review patch.  If anyone has any objections, please let me know.
> > >
> > > Why did you skip this patch for 4.11? IMO it should be applied there too.
> >
> > The thing is, this patch should probaly not even be in v4.12, as it should only
> > make any difference if there's a separate _bug_ in the kernel.
> >
> > This patch makes things more robust going forward, but I question that it needs to
> > be in -stable.
>
> Yeah, good point, I'm going to go drop it entirely from the 4.12-stable
> tree as it obviously isn't stable material, sorry for not catching that
> before.

Wait a minute. IIRC, Juergen told me last week that this patch fixes a bug
found/assigned by/to him. Juergen? If it is true then I would apply it to
stable. If I am wrong you can drop it.

Thanks,

Daniel

Re: [PATCH 4.12 26/84] x86/xen/efi: Initialize only the EFI struct members used by Xen

[Juergen Gross]

On 20/07/17 14:33, Daniel Kiper wrote:
> On Thu, Jul 20, 2017 at 11:16:39AM +0200, Greg Kroah-Hartman wrote:
>> On Thu, Jul 20, 2017 at 10:39:10AM +0200, Ingo Molnar wrote:
>>>
>>> * Daniel Kiper <daniel.kiper-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> wrote:
>>>
>>>> Hey Greg,
>>>>
>>>> On Wed, Jul 19, 2017 at 11:43:32AM +0200, Greg Kroah-Hartman wrote:
>>>>> 4.12-stable review patch.  If anyone has any objections, please let me know.
>>>>
>>>> Why did you skip this patch for 4.11? IMO it should be applied there too.
>>>
>>> The thing is, this patch should probaly not even be in v4.12, as it should only
>>> make any difference if there's a separate _bug_ in the kernel.
>>>
>>> This patch makes things more robust going forward, but I question that it needs to
>>> be in -stable.
>>
>> Yeah, good point, I'm going to go drop it entirely from the 4.12-stable
>> tree as it obviously isn't stable material, sorry for not catching that
>> before.
> 
> Wait a minute. IIRC, Juergen told me last week that this patch fixes a bug
> found/assigned by/to him. Juergen? If it is true then I would apply it to
> stable. If I am wrong you can drop it.

I'm not sure both patches (this one and upstream 457ea3f7e97881f) are
needed. I have got a report about a failing boot and with both patches
applied the boot is working. I will ask if 457ea3f7e97881f alone works,
too. If it doesn't I'll request this patch to be added to stable again.


Juergen

[PATCH 4.12 27/84] efi: Process the MEMATTR table only if EFI_MEMMAP is enabled

[Greg Kroah-Hartman]

4.12-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Daniel Kiper <daniel.kiper@oracle.com>

commit 457ea3f7e97881f937136ce0ba1f29f82b9abdb0 upstream.

Otherwise e.g. Xen dom0 on x86_64 EFI platforms crashes.

In theory we can check EFI_PARAVIRT too, however,
EFI_MEMMAP looks more targeted and covers more cases.

Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: andrew.cooper3@citrix.com
Cc: boris.ostrovsky@oracle.com
Cc: jgross@suse.com
Cc: linux-efi@vger.kernel.org
Cc: matt@codeblueprint.co.uk
Cc: xen-devel@lists.xenproject.org
Link: http://lkml.kernel.org/r/1498128697-12943-2-git-send-email-daniel.kiper@oracle.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 drivers/firmware/efi/efi.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -528,7 +528,8 @@ int __init efi_config_parse_tables(void
 		}
 	}
 
-	efi_memattr_init();
+	if (efi_enabled(EFI_MEMMAP))
+		efi_memattr_init();
 
 	/* Parse the EFI Properties table if it exists */
 	if (efi.properties_table != EFI_INVALID_TABLE_ADDR) {