[5/9] apm32: remove paravirt_enabled() use

Commit Message

Luis Chamberlain Feb. 19, 2016, 8:58 p.m. UTC

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):



> 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

Comments

Boris Ostrovsky Feb. 19, 2016, 10:17 p.m. UTC | #1

On 02/19/2016 03:58 PM, Luis R. Rodriguez wrote:
> On Fri, Feb 19, 2016 at 10:08:43AM -0500, Boris Ostrovsky wrote:
>
>> 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().

OK, I'll test it next week, it really should be done irrespective of 
what you are doing here.

-boris

Luis Chamberlain Feb. 20, 2016, 12:42 a.m. UTC | #2

On Fri, Feb 19, 2016 at 05:17:27PM -0500, Boris Ostrovsky wrote:
> 
> 
> On 02/19/2016 03:58 PM, Luis R. Rodriguez wrote:
> >On Fri, Feb 19, 2016 at 10:08:43AM -0500, Boris Ostrovsky wrote:
> >
> >>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().
> 
> OK, I'll test it next week, it really should be done irrespective of
> what you are doing here.

OK so it can go in separately?

  Luis

Boris Ostrovsky Feb. 22, 2016, 2:15 p.m. UTC | #3

On 02/19/2016 07:42 PM, Luis R. Rodriguez wrote:
> On Fri, Feb 19, 2016 at 05:17:27PM -0500, Boris Ostrovsky wrote:
>>
>> On 02/19/2016 03:58 PM, Luis R. Rodriguez wrote:
>>> On Fri, Feb 19, 2016 at 10:08:43AM -0500, Boris Ostrovsky wrote:
>>>
>>>> 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().
>> OK, I'll test it next week, it really should be done irrespective of
>> what you are doing here.
> OK so it can go in separately?


Yes, I'll send it as a separate patch.

-boris

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);