xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: "Luis R. Rodriguez" <mcgrof@kernel.org>
To: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: "Luis R. Rodriguez" <mcgrof@kernel.org>,
	bp@alien8.de, x86@kernel.org, linux-kernel@vger.kernel.org,
	luto@amacapital.net, rusty@rustcorp.com.au,
	david.vrabel@citrix.com, konrad.wilk@oracle.com,
	xen-devel@lists.xensource.com
Subject: Re: [PATCH 5/9] apm32: remove paravirt_enabled() use
Date: Fri, 19 Feb 2016 21:58:06 +0100	[thread overview]
Message-ID: <20160219205806.GS25240@wotan.suse.de> (raw)
In-Reply-To: <56C72FFB.5090105@oracle.com>

On Fri, Feb 19, 2016 at 10:08:43AM -0500, Boris Ostrovsky wrote:
> 
> 
> On 02/19/2016 08:08 AM, Luis R. Rodriguez wrote:
> >There is already a check for apm_info.bios == 0, the
> >apm_info.bios is set from the boot_params.apm_bios_info.
> >Both Xen and lguest, which are also the only ones that set
> >paravirt_enabled to true) do never set the apm_bios_info,
> >the paravirt_enabled() check is simply not needed.
> 
> We need to guarantee that boot_params is filled with zeroes. On
> baremetal path we clear .bss (which is where boot_params live)
> before copying data from zero page.

To be clear Xen is the only user of this that just sets it
outright instead of poking at the file and using that as
reference, and setting what it needs. Its a good point
though that things like this which are 0 still should
probably be set.

> So we need to at least memset(&boot_params, 0, sz)

This use case would be just Xen specific, for lguest
we can just set the things we need clearly on the
launcher. So like this (I can fold in a separate
patch):


diff --git a/tools/lguest/lguest.c b/tools/lguest/lguest.c
index ff0aa580c6e1..0aa75af6e862 100644
--- a/tools/lguest/lguest.c
+++ b/tools/lguest/lguest.c
@@ -3357,6 +3357,12 @@ int main(int argc, char *argv[])
 	/* Tell the entry path not to try to reload segment registers. */
 	boot->hdr.loadflags |= KEEP_SEGMENTS;
 
+	/* We don't support tboot */
+	boot->tboot_addr = 0;
+
+	/* Ensure this is 0 to prevent apm from loading */
+	boot->apm_bios_info.version = 0;
+
 	/* We tell the kernel to initialize the Guest. */
 	tell_kernel(start);
 

> in xen_start_kernel(). Better yet, clear whole .bss.
> 
> (This applies to the next patch as well).

So clear_bss() -- oh look, another call that xen_start_kernel() could have made
good use of. :)  Can you send a respective patch I can old into this series?
I'm afraid it is by no means obvious to me where it would be safe to do this on
xen_start_kernel().

  Luis

  reply	other threads:[~2016-02-19 20:58 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-19 13:08 [PATCH 0/9] x86/init: replace paravirt_enabled() were possible Luis R. Rodriguez
2016-02-19 13:08 ` [PATCH 1/9] x86/boot: enumerate documentation for the x86 hardware_subarch Luis R. Rodriguez
2016-02-19 13:19   ` [Xen-devel] " Juergen Gross
2016-02-19 13:40   ` David Vrabel
2016-02-19 14:40     ` Luis R. Rodriguez
2016-02-19 13:08 ` [PATCH 2/9] tools/lguest: make lguest launcher use X86_SUBARCH_LGUEST explicitly Luis R. Rodriguez
2016-02-19 13:08 ` [PATCH 3/9] x86/xen: use X86_SUBARCH_XEN for PV guest boots Luis R. Rodriguez
2016-02-19 13:08 ` [PATCH 4/9] x86/init: make ebda depend on PC subarch Luis R. Rodriguez
2016-02-19 13:08 ` [PATCH 5/9] apm32: remove paravirt_enabled() use Luis R. Rodriguez
2016-02-19 15:08   ` Boris Ostrovsky
2016-02-19 20:58     ` Luis R. Rodriguez [this message]
2016-02-19 22:17       ` Boris Ostrovsky
2016-02-20  0:42         ` Luis R. Rodriguez
2016-02-22 14:15           ` Boris Ostrovsky
2016-02-19 13:08 ` [PATCH 6/9] x86/tboot: remove paravirt_enabled() Luis R. Rodriguez
2016-02-19 13:08 ` [PATCH 7/9] x86/cpu/intel: replace paravirt_enabled() for f00f work around Luis R. Rodriguez
2016-02-19 13:08 ` [PATCH 8/9] x86/rtc: replace paravirt_enabled() check with subarch check Luis R. Rodriguez
2016-02-19 13:22   ` [Xen-devel] " Juergen Gross
2016-02-19 14:48     ` Luis R. Rodriguez
2016-02-22  6:07       ` Luis R. Rodriguez
2016-02-22 10:27         ` Borislav Petkov
2016-02-22 14:34           ` Boris Ostrovsky
2016-02-22 23:12             ` [Xen-devel] " Luis R. Rodriguez
2016-02-19 13:27   ` David Vrabel
2016-02-19 13:08 ` [PATCH 9/9] pnpbios: replace paravirt_enabled() check with subarch checks Luis R. Rodriguez
2016-02-19 13:34 ` [Xen-devel] [PATCH 0/9] x86/init: replace paravirt_enabled() were possible David Vrabel
2016-02-19 14:36   ` Luis R. Rodriguez

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160219205806.GS25240@wotan.suse.de \
    --to=mcgrof@kernel.org \
    --cc=boris.ostrovsky@oracle.com \
    --cc=bp@alien8.de \
    --cc=david.vrabel@citrix.com \
    --cc=konrad.wilk@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=rusty@rustcorp.com.au \
    --cc=x86@kernel.org \
    --cc=xen-devel@lists.xensource.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).