Re: [Xen-devel] [PATCH] x86/boot: fix MB2 header to require EFI BS

[Daniel Kiper]

On Tue, Oct 24, 2017 at 10:40:26PM +0100, Andrew Cooper wrote:
> On 24/10/2017 22:11, Daniel Kiper wrote:
> > On Tue, Oct 24, 2017 at 09:22:20PM +0100, Andrew Cooper wrote:
> >> On 24/10/17 21:08, Daniel Kiper wrote:
> >>> On Tue, Oct 24, 2017 at 02:40:41PM -0500, Doug Goldstein wrote:
> >>>> The EFI multiboot2 entry point currently requires EFI BootServices to
> >>>> not have been exited however the header currently tells the boot
> >>>> loader that Xen optionally supports EFI BootServices having been exited.
> >>>> With this change Xen properly advertises that EFI must not be exited
> >>>> allowing the boot loader to report an error that it cannot boot Xen if
> >>>> it is unable to meet its needs.
> >>>>
> >>>> Signed-off-by: Doug Goldstein <car...@cardoe.com>
> >>>> ---
> >>>>
> >>>> This should likely be applied against Xen 4.9 and Xen 4.10 as well as
> >>>> staging. I am trying to get multiboot2 support for iPXE and upstream
> >>>> is concerned that leaving EFI BootServices enabled will not be
> >>>> compatible with their aims to support Secure Boot. So when I build
> >>> Hmmm... What are exact arguments for that? How do they implement e.g.
> >>> chain loading then? What about the shim support?
> >>>
> >>>> my iPXE without support for passing on Boot Services, Xen will be
> >>>> loaded by iPXE but then it will fall down with "ERR: Bootloader
> >>>> shutdown EFI x64 boot services!" implying that this is required. By
> >>>> having Xen expose in its header that its required it allows me to
> >>>> handle the situation gracefully in iPXE.
> >>>>
> >>>> To quote the multiboot2 spec exact:
> >>>>
> >>>> "This tag indicates that payload supports starting without terminating
> >>>> boot services."
> >>>>
> >>>> Unfortunately the spec is a bit vague and how I am reading it is:
> >>>> - no tag = exit boot services in the boot loader
> >>>> - tag present marked optional = boot loader can or cannot exit boot 
> >>>> services
> >>>> - tag present marked required = boot loader cannot exit boot services
> >>> NACK, please take a look at section 3.1.4, Multiboot2 information request
> >>> in Multiboot2 spec. OPTIONAL/REQUIRED has different meaning for the 
> >>> bootloader
> >>> than you think.
> >> The meaning of tag, if understood by Grub, is "don't exit boot services
> >> before passing control".
> > Yep.
> >
> >> The tag is currently marked as optional, which means Grub is free to
> >> ignore it if it doesn't understand it, resulting in EBS being called
> >> before passing control.
> > Yep, but you are forgetting about legacy BIOS platforms with old GRUB2.
> > Right now it is possible to boot Xen via Multiboot2 in such configs.
> > If you set this flag to REQUIRED then old GRUB2 on BIOS platforms will
> > not boot Xen in such cases. If we do not care about that then OK. However,
> > then I would request additional line or so to the commit message saying
> > that such configs are broken deliberately because...
>
> Such older cases wouldn't boot either, because Xen would bail out saying
> "I didn't retain BS like I need".

Nope, you should remember that legacy entry point (__start) will be used then.

> >> Xen cannot cope with with EBS having been called, so must not be passed
> >> control under those circumstances.
> > Even with REQUIRED flag there is no guarantee that booloader will do
> > what Xen asks. So, it has to check the boot services presence anyway.
> > And it does.
>
> Indeed, and rightly so.
>
> The difference between Grub bailing out with an error, and Xen bailing
> out with an error, is that one still leaves you at a grub prompt, while
> one locks your system up until you remove some electrons from it.
>
> Setting the REQUIRED flag appears to be strictly better behaviour from
> the users point of view.

Yep, I put a solution proposal for that issue in other email.

Daniel

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

Re: [Xen-devel] [PATCH] x86/boot: fix MB2 header to require EFI BS

[Daniel Kiper]

On Tue, Oct 24, 2017 at 03:49:10PM -0500, Doug Goldstein wrote:
> On 10/24/17 3:22 PM, Andrew Cooper wrote:
> > On 24/10/17 21:08, Daniel Kiper wrote:
> >> On Tue, Oct 24, 2017 at 02:40:41PM -0500, Doug Goldstein wrote:
> >>> The EFI multiboot2 entry point currently requires EFI BootServices to
> >>> not have been exited however the header currently tells the boot
> >>> loader that Xen optionally supports EFI BootServices having been exited.
> >>> With this change Xen properly advertises that EFI must not be exited
> >>> allowing the boot loader to report an error that it cannot boot Xen if
> >>> it is unable to meet its needs.
> >>>
> >>> Signed-off-by: Doug Goldstein <car...@cardoe.com>
> >>> ---
> >>>
> >>> This should likely be applied against Xen 4.9 and Xen 4.10 as well as
> >>> staging. I am trying to get multiboot2 support for iPXE and upstream
> >>> is concerned that leaving EFI BootServices enabled will not be
> >>> compatible with their aims to support Secure Boot. So when I build
> >> Hmmm... What are exact arguments for that? How do they implement e.g.
> >> chain loading then? What about the shim support?
> >>
> >>> my iPXE without support for passing on Boot Services, Xen will be
> >>> loaded by iPXE but then it will fall down with "ERR: Bootloader
> >>> shutdown EFI x64 boot services!" implying that this is required. By
> >>> having Xen expose in its header that its required it allows me to
> >>> handle the situation gracefully in iPXE.
> >>>
> >>> To quote the multiboot2 spec exact:
> >>>
> >>> "This tag indicates that payload supports starting without terminating
> >>> boot services."
> >>>
> >>> Unfortunately the spec is a bit vague and how I am reading it is:
> >>> - no tag = exit boot services in the boot loader
> >>> - tag present marked optional = boot loader can or cannot exit boot 
> >>> services
> >>> - tag present marked required = boot loader cannot exit boot services
> >> NACK, please take a look at section 3.1.4, Multiboot2 information request
> >> in Multiboot2 spec. OPTIONAL/REQUIRED has different meaning for the 
> >> bootloader
> >> than you think.
> >
> > The meaning of tag, if understood by Grub, is "don't exit boot services
> > before passing control".
> >
> > The tag is currently marked as optional, which means Grub is free to
> > ignore it if it doesn't understand it, resulting in EBS being called
> > before passing control.
> >
> > Xen cannot cope with with EBS having been called, so must not be passed
> > control under those circumstances.
> >
> > Doug's patch marks it as non-optional which, by that section above,
> > requires Grub to fail with an error rather than proceeding, if it does
> > not understand the tag.
> >
> >
> > By my reading, Doug's patch looks correct.
> >
> > How does your interpretation of the spec differ?
> >
> > ~Andrew
> >
>
> So I've been sitting here reading it for a bit. I'm guessing what Daniel
> is arguing is that the spec says that the boot loader MUST understand a
> tag if its marked as required and does not have to understand it if its
> marked as optional. The next sentence then seems to be a total escape
> hatch because it seems to imply that the boot loader doesn't have to
> respect any tag regardless of its required or optional settings. Which
> seems to defeat the purpose of having any info requests at all. And
> results in no guarantees that if your binary requires something that it
> will get it before being executed. And therefore requires a binary to
> support all cases always.

Sadly you are right here.

> If that's truly the case you are arguing for Daniel then this whole spec
> really has too big of a loophole to be safely considered as useful. I
> know that's a bit harsh but as more tags are added over time the matrix
> of required support will snowball.

I think that we can use another bit from flags and if it set then
interpret it as: I (image/OS/Xen/...) have to have this data. If
you (the booloader) are not able to provide it then do not start
me and fail nicely, e.g. display user prompt. This bit should be
checked only if current bit is in REQUIRED state. Otherwise, if new
bit is in REQUIRED state and current one is in OPTIONAL state then
the bootloader should complain. This way older GRUB will fail if it
sees an unsupported option and newer one will always provide required
data to the loaded image. And by the way, we should check unused
flags bits for 1. If one of it is set then the bootloader should fail.
Right now at least GRUB2 does not do that.

Does it make sense?

Daniel

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

Re: [Xen-devel] [PATCH] x86/boot: fix MB2 header to require EFI BS

[Daniel Kiper]

On Tue, Oct 24, 2017 at 03:28:52PM -0500, Doug Goldstein wrote:
> On 10/24/17 3:08 PM, Daniel Kiper wrote:
> > On Tue, Oct 24, 2017 at 02:40:41PM -0500, Doug Goldstein wrote:
> >> The EFI multiboot2 entry point currently requires EFI BootServices to
> >> not have been exited however the header currently tells the boot
> >> loader that Xen optionally supports EFI BootServices having been exited.
> >> With this change Xen properly advertises that EFI must not be exited
> >> allowing the boot loader to report an error that it cannot boot Xen if
> >> it is unable to meet its needs.
> >>
> >> Signed-off-by: Doug Goldstein <car...@cardoe.com>
> >> ---
> >>
> >> This should likely be applied against Xen 4.9 and Xen 4.10 as well as
> >> staging. I am trying to get multiboot2 support for iPXE and upstream
> >> is concerned that leaving EFI BootServices enabled will not be
> >> compatible with their aims to support Secure Boot. So when I build
> >
> > Hmmm... What are exact arguments for that? How do they implement e.g.
> > chain loading then? What about the shim support?
>
> Look they have concerns about it. As we've talked about this in the past

If I do something I like to know why I have to do it.

> and I encourage you communicate with them. You are the author of the

I remember but, sorry, IIRC, I heard just only vague statements like that.
So, I would like to know exact reasons finally. And I hoped that they told
you more then simple "NO".

> multiboot2 spec. I'm just trying to do my best to PXE boot Xen on EFI
> systems and make all upstreams (Xen & iPXE) happy.

Once again, I am happy to help. Though I have to know why I have to do
this or that. No more no less.

> >> Unfortunately the spec is a bit vague and how I am reading it is:
> >> - no tag = exit boot services in the boot loader
> >> - tag present marked optional = boot loader can or cannot exit boot 
> >> services
> >> - tag present marked required = boot loader cannot exit boot services
> >
> > NACK, please take a look at section 3.1.4, Multiboot2 information request
> > in Multiboot2 spec. OPTIONAL/REQUIRED has different meaning for the 
> > bootloader
> > than you think.
>
> I still don't see any issue with my interpretation based on what you
> pointed me to. There's a hole here with what Xen asks for of the boot
> loader to do.
>
> The boot loader is told that Xen optionally supports the boot loader not
> exiting boot services when in fact Xen requires the boot loader to not
> exit boot services. Somehow we need to convey this to the boot loader.

Sorry, maybe I was too vague this time. Please look at my replay to Andrew.
It should help.

Daniel

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

Re: [Xen-devel] [PATCH] x86/boot: fix MB2 header to require EFI BS

[Daniel Kiper]

On Tue, Oct 24, 2017 at 09:22:20PM +0100, Andrew Cooper wrote:
> On 24/10/17 21:08, Daniel Kiper wrote:
> > On Tue, Oct 24, 2017 at 02:40:41PM -0500, Doug Goldstein wrote:
> >> The EFI multiboot2 entry point currently requires EFI BootServices to
> >> not have been exited however the header currently tells the boot
> >> loader that Xen optionally supports EFI BootServices having been exited.
> >> With this change Xen properly advertises that EFI must not be exited
> >> allowing the boot loader to report an error that it cannot boot Xen if
> >> it is unable to meet its needs.
> >>
> >> Signed-off-by: Doug Goldstein <car...@cardoe.com>
> >> ---
> >>
> >> This should likely be applied against Xen 4.9 and Xen 4.10 as well as
> >> staging. I am trying to get multiboot2 support for iPXE and upstream
> >> is concerned that leaving EFI BootServices enabled will not be
> >> compatible with their aims to support Secure Boot. So when I build
> > Hmmm... What are exact arguments for that? How do they implement e.g.
> > chain loading then? What about the shim support?
> >
> >> my iPXE without support for passing on Boot Services, Xen will be
> >> loaded by iPXE but then it will fall down with "ERR: Bootloader
> >> shutdown EFI x64 boot services!" implying that this is required. By
> >> having Xen expose in its header that its required it allows me to
> >> handle the situation gracefully in iPXE.
> >>
> >> To quote the multiboot2 spec exact:
> >>
> >> "This tag indicates that payload supports starting without terminating
> >> boot services."
> >>
> >> Unfortunately the spec is a bit vague and how I am reading it is:
> >> - no tag = exit boot services in the boot loader
> >> - tag present marked optional = boot loader can or cannot exit boot 
> >> services
> >> - tag present marked required = boot loader cannot exit boot services
> > NACK, please take a look at section 3.1.4, Multiboot2 information request
> > in Multiboot2 spec. OPTIONAL/REQUIRED has different meaning for the 
> > bootloader
> > than you think.
>
> The meaning of tag, if understood by Grub, is "don't exit boot services
> before passing control".

Yep.

> The tag is currently marked as optional, which means Grub is free to
> ignore it if it doesn't understand it, resulting in EBS being called
> before passing control.

Yep, but you are forgetting about legacy BIOS platforms with old GRUB2.
Right now it is possible to boot Xen via Multiboot2 in such configs.
If you set this flag to REQUIRED then old GRUB2 on BIOS platforms will
not boot Xen in such cases. If we do not care about that then OK. However,
then I would request additional line or so to the commit message saying
that such configs are broken deliberately because...

> Xen cannot cope with with EBS having been called, so must not be passed
> control under those circumstances.

Even with REQUIRED flag there is no guarantee that booloader will do
what Xen asks. So, it has to check the boot services presence anyway.
And it does.

Daniel

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

Re: [Xen-devel] [PATCH] x86/boot: fix MB2 header to require EFI BS

[Daniel Kiper]

On Tue, Oct 24, 2017 at 02:40:41PM -0500, Doug Goldstein wrote:
> The EFI multiboot2 entry point currently requires EFI BootServices to
> not have been exited however the header currently tells the boot
> loader that Xen optionally supports EFI BootServices having been exited.
> With this change Xen properly advertises that EFI must not be exited
> allowing the boot loader to report an error that it cannot boot Xen if
> it is unable to meet its needs.
>
> Signed-off-by: Doug Goldstein 
> ---
>
> This should likely be applied against Xen 4.9 and Xen 4.10 as well as
> staging. I am trying to get multiboot2 support for iPXE and upstream
> is concerned that leaving EFI BootServices enabled will not be
> compatible with their aims to support Secure Boot. So when I build

Hmmm... What are exact arguments for that? How do they implement e.g.
chain loading then? What about the shim support?

> my iPXE without support for passing on Boot Services, Xen will be
> loaded by iPXE but then it will fall down with "ERR: Bootloader
> shutdown EFI x64 boot services!" implying that this is required. By
> having Xen expose in its header that its required it allows me to
> handle the situation gracefully in iPXE.
>
> To quote the multiboot2 spec exact:
>
> "This tag indicates that payload supports starting without terminating
> boot services."
>
> Unfortunately the spec is a bit vague and how I am reading it is:
> - no tag = exit boot services in the boot loader
> - tag present marked optional = boot loader can or cannot exit boot services
> - tag present marked required = boot loader cannot exit boot services

NACK, please take a look at section 3.1.4, Multiboot2 information request
in Multiboot2 spec. OPTIONAL/REQUIRED has different meaning for the bootloader
than you think.

> In the future I would like to add support to Xen to allow it to run
> without boot services but presently that support isn't there.

I tried that. This is difficult but not impossible. Hmmm... IIRC, some
things are impossible. Please take a look at efi_multiboot2() and you
quickly will know. Though why not try again.

Daniel

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

Re: [Xen-devel] [PATCH v2 2/2] x86/boot: rename send_chr to print_err

[Daniel Kiper]

On Tue, Oct 17, 2017 at 04:41:38PM -0500, Doug Goldstein wrote:
> From: David Esler <drumandst...@gmail.com>
>
> The send_chr function sends an entire C-string and not one character and
> doesn't necessarily just send it over the serial UART anymore so rename
> it to print_err so that its closer in name to what it does.
>
> Reviewed-by: Doug Goldstein <car...@cardoe.com>

Ditto.

> Signed-off-by: David Esler <drumandst...@gmail.com>

Anyway, Reviewed-by: Daniel Kiper <daniel.ki...@oracle.com>

Daniel

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

Re: [Xen-devel] [PATCH v2 1/2] x86/boot: fix early error display

[Daniel Kiper]

On Tue, Oct 17, 2017 at 04:41:37PM -0500, Doug Goldstein wrote:
> From: David Esler <drumandst...@gmail.com>
>
> In 9180f5365524 a change was made to the send_chr function to take in
> C-strings and print out a character at a time until a NULL was
> encountered. However there is no code to increment the current character
> position resulting in an endless loop of the first character. This adds
> a simple increment.
>
> Reviewed-by: Doug Goldstein <car...@cardoe.com>

I was told that "Reviewed-by: ..." should be after SOB.

> Signed-off-by: David Esler <drumandst...@gmail.com>

In general Reviewed-by: Daniel Kiper <daniel.ki...@oracle.com>
Though please take into account Jan's request WRT to commit
message. Or I am OK with Jan's changes before committing.

Daniel

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

Re: [Xen-devel] [Xen-users] UEFI Secure Boot Xen 4.9

[Daniel Kiper]

On Thu, Oct 12, 2017 at 05:03:13PM +, Bill Jacobs (billjac) wrote:
> Hi
> What is the status of creating a shim to abstract secure boot
> signing for Xen (to leverage MSFT 3rd party, e.g)?

xen.efi works with shim itself out of the box. If you wish
to use shim and GRUB2 to load Xen you have to look at these
RFC patch series:
  - https://lists.xen.org/archives/html/xen-devel/2017-07/msg00982.html
  - https://lists.xen.org/archives/html/xen-devel/2017-07/msg00985.html

I am slowly going back to this work. So, I hope that it will
be taken into Xen 4.11 or so.

Daniel

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

Re: [Xen-devel] [PATCH 2/2] x86/boot: rename send_chr to print_err

[Daniel Kiper]

On Thu, Oct 12, 2017 at 09:56:01PM +0100, Andrew Cooper wrote:
> On 12/10/2017 21:50, Doug Goldstein wrote:
> > From: David Esler <drumandst...@gmail.com>
> >
> > The send_chr function sends an entire C-string and not one character and
> > doesn't necessarily just send it over the serial UART anymore so rename
> > it to print_err so that its closer in name to what it does.
> >
> > Reviewed-by: Doug Goldstein <car...@cardoe.com>
> > Signed-off-by: David Esler <drumandst...@gmail.com>
>
> Reviewed-by: Andrew Cooper <andrew.coop...@citrix.com>

Reviewed-by: Daniel Kiper <daniel.ki...@oracle.com>

Daniel

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

Re: [Xen-devel] [PATCH 1/2] x86/boot: fix early error display

[Daniel Kiper]

On Thu, Oct 12, 2017 at 03:50:06PM -0500, Doug Goldstein wrote:
> From: David Esler 
>
> In 9180f5365524 a change was made to the send_chr function to take in
> C-strings and print out a character at a time until a NULL was
> encountered. However there is no code to increment the current character
> position resulting in an endless loop of the first character. This adds
> a simple increment.
>
> Reviewed-by: Doug Goldstein 
> Signed-off-by: David Esler 
> ---
>  xen/arch/x86/boot/head.S | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
> index fd6fc337fe..f48bbbd2e5 100644
> --- a/xen/arch/x86/boot/head.S
> +++ b/xen/arch/x86/boot/head.S
> @@ -174,6 +174,7 @@ not_multiboot:
>  mov sym_esi(vga_text_buffer),%edi
>  .Lsend_chr:
>  mov (%esi),%bl
> +inc %esi
>  test%bl,%bl# Terminate on '\0' sentinel
>  je  .Lhalt
>  mov $0x3f8+5,%dx   # UART Line Status Register

I have a feeling that you have tested this on machine without
VGA text buffer available. Then your fix works. However, if VGA
text buffer is available then %esi is increased twice. First time
by inc here and once again by movsb below. So, I think that the
issue have to be fixed in a bit different way.

Daniel

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

Re: [Xen-devel] Booting signed xen.efi through shim

[Daniel Kiper]

On Fri, Sep 22, 2017 at 02:25:46AM -0600, Jan Beulich wrote:
> >>> On 22.09.17 at 00:46,  wrote:
> > One piece that I see still missing is the Xen command line parameters
> > not being verified. It would be ideal to have the option to get that
> > set during compile time as well, similar to Linux's CONFIG_CMDLINE
> > option, to avoid for example getting iommu or XSM being turned off by
> > someone with physical access.
>
> We do have CMDLINE and CMDLINE_OVERRIDE. But for someone
> with physical access it would likely also be possible to avoid secure
> boot altogether?

Another solutions is here: 
http://lists.gnu.org/archive/html/grub-devel/2017-07/msg3.html
It is TPM based and WIP. It requires verifiers framework which should
be posted on grub-devel soon. Or you can add your own method based
on verifiers. Patches are welcome...

Have a nice weekend,

Daniel

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

Re: [Xen-devel] Booting signed xen.efi through shim

[Daniel Kiper]

On Wed, Sep 20, 2017 at 09:59:51AM -0600, Tamas K Lengyel wrote:
> On Wed, Sep 20, 2017 at 9:46 AM, Jan Beulich  wrote:
>  On 20.09.17 at 17:20,  wrote:
> >> On Wed, Sep 20, 2017 at 12:30 AM, Jan Beulich  wrote:
> >> On 20.09.17 at 00:23,  wrote:
>  Yeap, the shim pretty simply removed the .reloc section as it was
>  marked discardable and did the relocations for Xen. So with that
>  removed from the shim I no longer get the error and I see that the
>  dom0 kernel gets verified using the shim lock protocol.
> >>>
> >>> So did you instead try whether simply clearing the discardable
> >>> flag from the section also helps? The flag ought to matter in
> >>> paged environments only (which EFI isn't despite paging being
> >>> enabled), but as we see some people think otherwise.
> >>
> >> Yes, that would work. Even if the shim does its relocations everything
> >> "just works" as long as Xen can also find the .reloc section. For now
> >> it was just simpler for me to patch the shim to copy the reloc section
> >> but it would be ideal if the xen.efi that's produced during
> >> compilation would not have that flag set to begin with. I did search
> >> around briefly to see where that flag is coming from but the only
> >> reference to it within Xen I found was arch/x86/efi/mkreloc.c. So sans
> >> writing a separate tool that binary patches xen.efi after compilation
> >> to remove that flag I'm not sure how to get that done.
> >
> > That'll likely be another binutils tweak. What I find odd is that you're
> > apparently the only one to have this problem.
>
> My guess is that not many people have actually tried booting Xen
> through the shim before. I looked through the shim history and the
> reloc section was discarded all the way back to several years if it
> was marked discardable. So unless the discardable flag is something
> that only has been added to the reloc section recently, someone should
> have run into it already.

I have played with Xen master in July and have not seen any issues.
I will take a stab at it probably next week.

>  I still didn't
>  get dom0 to boot for some reason but that might be an unrelated issue
>  (and I have no serial console right now). Nevertheless, progress!
> >>>
> >>> And it doesn't get far enough for you to see any output at all?
> >>> Did you try "earlyprintk=xen" on the kernel command line and/or
> >>> "vga=keep" on the Xen one?
> >>
> >> I tried with both just now, no output at all from Xen on the screen
> >> after it exits EFI boot. I also couldn't get any output from it on my
> >> other laptop with Intel AMT.
> >
> > Odd.
> >
> >> I did manage to get another Linux kernel booting but my goal was to
> >> get a shim verified dom0 kernel booting without an initrd image as
> >> right now the ramdisk is not verified by the shim (also not sure how
> >> that's supposed to work as sbsign/pesign can only deal with PE/COFF
> >> binaries which the ramdisk isn't).
> >
> > That's not how it's supposed to work, I think. Just like the shim only
> > verifies Xen and hands through the other modules unchecked, Xen
> > only verifies the Dom0 kernel image (with the help of the shim). It's
> > the Dom0 kernel to then verify the content (not necessarily the
> > blob) of the initrd.
> >
>
> Yea, that would be a sensible approach though I haven't (yet) found
> anything that I could use with linux to do that initrd verification.
> So my approach right now is to get the initrd baked into the dom0
> kernel, that way the whole thing can be signed and Xen can verify both
> in one shot using shim.

Partial solution for your problem is Linux kernel module signing.

Daniel

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

Re: [Xen-devel] [Xen-users] UEFI Secure Boot Xen 4.9

[Daniel Kiper]

On Mon, Sep 18, 2017 at 11:24:15AM -0400, Tamas K Lengyel wrote:
> On Tue, Sep 5, 2017 at 12:26 PM, Tamas K Lengyel
> <tamas.k.leng...@gmail.com> wrote:
> > On Mon, Sep 4, 2017 at 6:40 AM, Daniel Kiper <daniel.ki...@oracle.com> 
> > wrote:
> >> On Wed, Aug 30, 2017 at 10:16:23AM -0600, Tamas K Lengyel wrote:
> >>> On Tue, Aug 29, 2017 at 2:01 PM, Daniel Kiper <daniel.ki...@oracle.com> 
> >>> wrote:
> >>> > Hey Tamas,
> >>> >
> >>> > Sorry for late reply. I was on vacation.
> >>> >
> >>> > On Tue, Aug 22, 2017 at 09:01:06PM -0600, Tamas K Lengyel wrote:
> >>> >> On Tue, May 16, 2017 at 5:04 AM, Daniel Kiper 
> >>> >> <daniel.ki...@oracle.com> wrote:
> >>> >
> >>> > [...]
> >>> >
> >>> >> > UEFI will verify shim secure boot signature then shim will verify 
> >>> >> > GRUB2
> >>> >> > signature then GRUB2 will verify (with shim protocol) Xen signature 
> >>> >> > and
> >>> >> > finally Xen will verify (with shim protocol) Linux kernel signature. 
> >>> >> > Then
> >>> >> > your kernel can verify modules using whatever you want.
> >>> >> >
> >>> >> >> I would be happy to work to help achieve this.
> >>> >> >
> >>> >> > There is a chance that I will have something very raw at the 
> >>> >> > beginning
> >>> >> > of June. If you wish to do tests drop me a line.
> >>> >>
> >>> >> Hi Daniel,
> >>> >> is there any news on this? I would be interested in giving this a shot 
> >>> >> too.
> >>> >
> >>> > Please look at
> >>> >
> >>> >   https://lists.xen.org/archives/html/xen-devel/2017-07/msg00982.html
> >>> >
> >>> > and at
> >>> >
> >>> >   https://lists.xen.org/archives/html/xen-devel/2017-07/msg00985.html
> >>> >
> >>> > Attachments contain the same patches as above but rebased on latest
> >>> > GRUB2 and Xen git repositories.
> >>> >
> >>> > Due to some travel I am going to restart work on this in the second
> >>> > half of September.
> >>> >
> >>> > If you have any questions please drop me a line.
> >>> >
> >>>
> >>> Hi Daniel,
> >>> thanks for the update, I'll give it a shot today to set it up. In a
> >>> somewhat related note, are you aware of any work on getting secure
> >>> boot + UEFI working in a guest? There is a PoC patch on OpenXT
> >>> (https://github.com/OpenXT/xenclient-oe/pull/729) but was wondering if
> >>> there are any parallel efforts ongoing.
> >>
> >> I do not follow this issue in detail. However, I suppose that if OVMF
> >> supports UEFI secure boot (well, QEMU has to enable SMM support too;
> >> I do not know does it work with Xen or not) then guest should work
> >> without any issue. Just guessing...
> >>
> >
> > Sure, was just wondering if you are aware of anyone looking at that.
> >
> > In other news I was able to get your patches working and have been
> > able to boot with Secure boot enabled as far as shim -> signed grub ->
> > signed linux without initrd. If I boot a signed version of Xen from
> > grub it goes as far as setup_efi_pci but then the system reboots
> > without anything else being printed on the screen. I haven't been able
> > to debug it any further yet.
> >
>
> Daniel,
> just FYI the xen.mb.efi generated with your patches causes pesign to segfault:
>
> cms_pe_common.c:generate_digest:198 PE section ".text" has invalid address
> Segmentation fault

Thank you for doing the tests. I am going to restart work on this next week
and post next version of patches in October. I will try to fix all issues
spotted by you. Stay tuned...

Daniel

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

Re: [Xen-devel] Booting signed xen.efi through shim

[Daniel Kiper]

Hi Tamas,

On Tue, Sep 12, 2017 at 05:40:35PM -0600, Tamas K Lengyel wrote:
> Hi all,
> for the last couple weeks I've been poking around the options
> available to get Xen booted on a Secureboot enabled box. My goal is to
> extend the chain of trust to the dom0 kernel. According to
> https://wiki.xenproject.org/wiki/Xen_EFI this is something that's
> supposed to be supported out-of-the-box right now via the shim
> protocol. However, when I try to boot a signed xen.efi (4.10 unstable
> head) through shim I get the error "Section 6 is inside image header"

Strange... Could you send more info about your environment?
C compiler type, its version, binutils version, etc. How
did you sign xen.efi? Which tool you used to do that?
Have you seen any warnings or errors during sign?

> and shim refuses to load Xen. OTOH I had been able to boot a
> custom-compiled grub2 from the shim no problem with Secureboot

What do you mean by "custom-compiled grub2"?

> enabled. The signed xen.efi also boots fine with Secureboot enabled if
> booted directly as an EFI application (but then no dom0 verification

IIRC, shim is very picky with PE format. So, anything which is loaded
by EFI loader may not be loaded by shim.

> is done AFAIU). Does anyone have any pointers on what's going on with

Right, only shim provides a such functionality.

> booting through the shim?

I am happy to help but in cases like that I need more info, e.g.: serial
console logs, output from "objdump -x xen/xen.efi" command, etc.

Daniel

PS I am traveling, so, I am reading my emails from time to time.

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

Re: [Xen-devel] [PATCH v2] Fix ARM multiboot2 breaking Fedora.

[Daniel Kiper]

On Wed, Aug 30, 2017 at 12:26:28PM +0200, Daniel Kiper wrote:
> On Tue, Aug 29, 2017 at 04:40:51PM -0400, Konrad Rzeszutek Wilk wrote:
> > Since v1 
> > [http://lists.gnu.org/archive/html/grub-devel/2017-08/msg00073.html]
> >  - Fixed up patch with failing invocation,
> >  - Redid patch #2 per Daniel's instructions.
> >
> >
> > Hey,
> >
> > The first patch:
> >  [PATCH 1/2] Fix util/grub.d/20_linux_xen.in: Add xen_boot command
> >
> > is a fix discovered on Fedora rawhide where I was surprised to see that
> > grub2-mkconfig would not create a configuration file anymore.
> >
> > The second patch has been posted in the past
> > (https://lists.xen.org/archives/html/xen-devel/2017-03/txtCeHTNmz1hZ.txt)
> >
> >  [PATCH 2/2] Use grub-file to figure out whether multiboot2 should be
> >
> > and just allows us to multiboot2 instead of multiboot if the binary
> > supports it.
>
> LGMT. If there are no objections in a week or so I will apply both of them.

Applied!

Thanks,

Daniel

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

[Xen-devel] GRUB documentation updated

[Daniel Kiper]

Hey,

Some people asked me about Multiboot2 Specification and other GRUB doc stuff.
So, I have put latest things at
  https://www.gnu.org/software/grub/grub-documentation.html

I hope that helps. If you have any questions please drop me a line.

Thanks,

Daniel

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

Re: [Xen-devel] [Xen-users] UEFI Secure Boot Xen 4.9

[Daniel Kiper]

On Wed, Aug 30, 2017 at 10:16:23AM -0600, Tamas K Lengyel wrote:
> On Tue, Aug 29, 2017 at 2:01 PM, Daniel Kiper <daniel.ki...@oracle.com> wrote:
> > Hey Tamas,
> >
> > Sorry for late reply. I was on vacation.
> >
> > On Tue, Aug 22, 2017 at 09:01:06PM -0600, Tamas K Lengyel wrote:
> >> On Tue, May 16, 2017 at 5:04 AM, Daniel Kiper <daniel.ki...@oracle.com> 
> >> wrote:
> >
> > [...]
> >
> >> > UEFI will verify shim secure boot signature then shim will verify GRUB2
> >> > signature then GRUB2 will verify (with shim protocol) Xen signature and
> >> > finally Xen will verify (with shim protocol) Linux kernel signature. Then
> >> > your kernel can verify modules using whatever you want.
> >> >
> >> >> I would be happy to work to help achieve this.
> >> >
> >> > There is a chance that I will have something very raw at the beginning
> >> > of June. If you wish to do tests drop me a line.
> >>
> >> Hi Daniel,
> >> is there any news on this? I would be interested in giving this a shot too.
> >
> > Please look at
> >
> >   https://lists.xen.org/archives/html/xen-devel/2017-07/msg00982.html
> >
> > and at
> >
> >   https://lists.xen.org/archives/html/xen-devel/2017-07/msg00985.html
> >
> > Attachments contain the same patches as above but rebased on latest
> > GRUB2 and Xen git repositories.
> >
> > Due to some travel I am going to restart work on this in the second
> > half of September.
> >
> > If you have any questions please drop me a line.
> >
>
> Hi Daniel,
> thanks for the update, I'll give it a shot today to set it up. In a
> somewhat related note, are you aware of any work on getting secure
> boot + UEFI working in a guest? There is a PoC patch on OpenXT
> (https://github.com/OpenXT/xenclient-oe/pull/729) but was wondering if
> there are any parallel efforts ongoing.

I do not follow this issue in detail. However, I suppose that if OVMF
supports UEFI secure boot (well, QEMU has to enable SMM support too;
I do not know does it work with Xen or not) then guest should work
without any issue. Just guessing...

Daniel

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

Re: [Xen-devel] [PATCH v2] Fix ARM multiboot2 breaking Fedora.

[Daniel Kiper]

On Tue, Aug 29, 2017 at 04:40:51PM -0400, Konrad Rzeszutek Wilk wrote:
> Since v1 [http://lists.gnu.org/archive/html/grub-devel/2017-08/msg00073.html]
>  - Fixed up patch with failing invocation,
>  - Redid patch #2 per Daniel's instructions.
>
>
> Hey,
>
> The first patch:
>  [PATCH 1/2] Fix util/grub.d/20_linux_xen.in: Add xen_boot command
>
> is a fix discovered on Fedora rawhide where I was surprised to see that
> grub2-mkconfig would not create a configuration file anymore.
>
> The second patch has been posted in the past
> (https://lists.xen.org/archives/html/xen-devel/2017-03/txtCeHTNmz1hZ.txt)
>
>  [PATCH 2/2] Use grub-file to figure out whether multiboot2 should be
>
> and just allows us to multiboot2 instead of multiboot if the binary
> supports it.

LGMT. If there are no objections in a week or so I will apply both of them.

Daniel

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

Re: [Xen-devel] [Xen-users] UEFI Secure Boot Xen 4.9

[Daniel Kiper]

Hey Tamas,

Sorry for late reply. I was on vacation.

On Tue, Aug 22, 2017 at 09:01:06PM -0600, Tamas K Lengyel wrote:
> On Tue, May 16, 2017 at 5:04 AM, Daniel Kiper <daniel.ki...@oracle.com> wrote:

[...]

> > UEFI will verify shim secure boot signature then shim will verify GRUB2
> > signature then GRUB2 will verify (with shim protocol) Xen signature and
> > finally Xen will verify (with shim protocol) Linux kernel signature. Then
> > your kernel can verify modules using whatever you want.
> >
> >> I would be happy to work to help achieve this.
> >
> > There is a chance that I will have something very raw at the beginning
> > of June. If you wish to do tests drop me a line.
>
> Hi Daniel,
> is there any news on this? I would be interested in giving this a shot too.

Please look at

  https://lists.xen.org/archives/html/xen-devel/2017-07/msg00982.html

and at

  https://lists.xen.org/archives/html/xen-devel/2017-07/msg00985.html

Attachments contain the same patches as above but rebased on latest
GRUB2 and Xen git repositories.

Due to some travel I am going to restart work on this in the second
half of September.

If you have any questions please drop me a line.

Daniel
>From 8458d7904886ca4bea059d103dac2ba50e53c13b Mon Sep 17 00:00:00 2001
From: Daniel Kiper <daniel.ki...@oracle.com>
Date: Sat, 8 Jul 2017 23:32:36 +0200
Subject: [PATCH] efi: Add EFI shim lock verifier

This is based on git://git.savannah.gnu.org/grub.git phcoder/verifiers branch.

Just an RFC.

TODO:
  - disable the GRUB2 modules load/unload,
  - disable the dangerous modules, e.g. iorw, memrw.

Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com>
---
 grub-core/Makefile.core.def|6 +++
 grub-core/commands/efi/shim_lock.c |  100 
 2 files changed, 106 insertions(+)
 create mode 100644 grub-core/commands/efi/shim_lock.c

diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index 16c4d0e..c38e4a8 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -905,6 +905,12 @@ module = {
 };
 
 module = {
+  name = shim_lock;
+  common = commands/efi/shim_lock.c;
+  enable = x86_64_efi;
+};
+
+module = {
   name = hdparm;
   common = commands/hdparm.c;
   common = lib/hexdump.c;
diff --git a/grub-core/commands/efi/shim_lock.c b/grub-core/commands/efi/shim_lock.c
new file mode 100644
index 000..40d2b25
--- /dev/null
+++ b/grub-core/commands/efi/shim_lock.c
@@ -0,0 +1,100 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2017  Free Software Foundation, Inc.
+ *
+ *  GRUB is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  GRUB is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ *  EFI shim lock verifier.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+GRUB_MOD_LICENSE ("GPLv3+");
+
+#define GRUB_EFI_SHIM_LOCK_GUID \
+  { 0x605dab50, 0xe046, 0x4300, \
+{ 0xab, 0xb6, 0x3d, 0xd8, 0x10, 0xdd, 0x8b, 0x23 } \
+  }
+
+struct grub_efi_shim_lock_protocol
+{
+  grub_efi_status_t
+  (*verify) (void *buffer,
+	 grub_uint32_t size);
+};
+typedef struct grub_efi_shim_lock_protocol grub_efi_shim_lock_protocol_t;
+
+static grub_efi_guid_t shim_lock_guid = GRUB_EFI_SHIM_LOCK_GUID;
+static grub_efi_shim_lock_protocol_t *sl;
+
+static grub_err_t
+shim_lock_init (grub_file_t io __attribute__ ((unused)), enum grub_file_type type,
+	   void **context __attribute__ ((unused)), enum grub_verify_flags *flags)
+{
+  *flags = GRUB_VERIFY_FLAGS_SKIP_VERIFICATION;
+
+  if (!sl)
+return GRUB_ERR_NONE;
+
+  switch (type & GRUB_FILE_TYPE_MASK)
+{
+case GRUB_FILE_TYPE_LINUX_KERNEL:
+case GRUB_FILE_TYPE_MULTIBOOT_KERNEL:
+case GRUB_FILE_TYPE_BSD_KERNEL:
+case GRUB_FILE_TYPE_XNU_KERNEL:
+case GRUB_FILE_TYPE_PLAN9_KERNEL:
+  *flags = GRUB_VERIFY_FLAGS_SINGLE_CHUNK;
+
+default:
+  return GRUB_ERR_NONE;
+}
+}
+
+static grub_err_t
+shim_lock_write (void *context __attribute__ ((unused)), void *buf, grub_size_t size)
+{
+  if (sl->verify (buf, size) != GRUB_EFI_SUCCESS)
+return grub_error (GRUB_ERR_BAD_SIGNATURE, N_("bad shim signature"));
+
+  return GRUB_ERR_NONE;
+}
+
+static void
+shim_lock_close (void *context __attribute__ ((unused)))
+{
+}
+
+struct grub_file_verifier shim_lock =
+  {
+.name = "shim_lock",
+.init = shim_lock_ini

Re: [Xen-devel] [PATCH 2/2] Use grub-file to figure out whether multiboot2 should be used for Xen.gz

[Daniel Kiper]

On Mon, Aug 28, 2017 at 02:40:15PM -0400, Konrad Rzeszutek Wilk wrote:
> The multiboot2 is much more preferable than multiboot. Especiall
> if booting under EFI where multiboot does not have the functionality
> to pass ImageHandler.
>
> Signed-off-by: Konrad Rzeszutek Wilk 
> ---
> v2: Rebase on top of  d33045ce7ffcb7c1e4a60c14d5ca64b36e3c5abe
> ---
>  util/grub.d/20_linux_xen.in | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/util/grub.d/20_linux_xen.in b/util/grub.d/20_linux_xen.in
> index 083bcef..29e015b 100644
> --- a/util/grub.d/20_linux_xen.in
> +++ b/util/grub.d/20_linux_xen.in
> @@ -212,6 +212,10 @@ while [ "x${xen_list}" != "x" ] ; do
>  else
>   xen_loader="multiboot"
>   module_loader="module"

Could you put these two lines into else below?

> + if ($grub_file --is-x86-multiboot2 $current_xen); then
> + xen_loader="multiboot2"
> + module_loader="module2"

Too much tabs. It should be one tab and four spaces.

Daniel

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

Re: [Xen-devel] [PATCH 1/2] Fix util/grub.d/20_linux_xen.in: Add xen_boot command support for aarch64

[Daniel Kiper]

On Mon, Aug 28, 2017 at 02:42:18PM -0400, Konrad Rzeszutek Wilk wrote:
> On Mon, Aug 28, 2017 at 02:40:14PM -0400, Konrad Rzeszutek Wilk wrote:
> > Commit d33045ce7ffcb7c1e4a60c14d5ca64b36e3c5abe introduced
> > the support for this, but it does not work under x86 (as it stops
> > 20_linux_xen from running).
> >
> > The 20_linux_xen is run under a shell and any exits from within it:
> >
> > (For example on x86):
> > + /usr/bin/grub2-file --is-arm64-efi /boot/xen-4.9.0.gz
> > [root@tst063 grub]# echo $?
> > 1
> >
> > will result in 20_linux_xen exciting without continuing

s/exciting/exiting/?

> > and also causing grub2-mkconfig to stop processing.
> >
> > As in:
>
> git format-patch decided to eat this relevant part:
>
> [root@tst063 grub]# ./grub-mkconfig | tail
> Generating grub configuration file ...
> Found linux image: /boot/vmlinuz-4.13.0-0.rc5.git1.1.fc27.x86_64
> Found initrd image: /boot/initramfs-4.13.0-0.rc5.git1.1.fc27.x86_64.img
> Found linux image: /boot/vmlinuz-0-rescue-ec082ee24aea41b9b16aca52a6d10cc2
> Found initrd image: 
> /boot/initramfs-0-rescue-ec082ee24aea41b9b16aca52a6d10cc2.img
>   echo'Loading Linux 
> 0-rescue-ec082ee24aea41b9b16aca52a6d10cc2 ...'
>   linux   /vmlinuz-0-rescue-ec082ee24aea41b9b16aca52a6d10cc2 
> root=/dev/mapper/fedora_tst063-root ro single
>   echo'Loading initial ramdisk ...'
>   initrd  /initramfs-0-rescue-ec082ee24aea41b9b16aca52a6d10cc2.img
>   }
> }
>
> ### END /usr/local/etc/grub.d/10_linux ###
>
> ### BEGIN /usr/local/etc/grub.d/20_linux_xen ###
>
> root@tst063 grub]#

In general LGTM. Though please repost this patch with full commit message?

Daniel

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

Re: [Xen-devel] [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.ki...@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.

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

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

Re: [Xen-devel] Xen 4.10 Development Update

[Daniel Kiper]

Hey Julien,

On Mon, Jul 17, 2017 at 02:26:22PM +0100, Julien Grall wrote:
> This email only tracks big items for xen.git tree. Please reply for items you
> woulk like to see in 4.10 so that people have an idea what is going on and
> prioritise accordingly.
>
> You're welcome to provide description and use cases of the feature you're
> working on.
>
> = Timeline =
>
> We now adopt a fixed cut-off date scheme. We will release twice a
> year. The upcoming 4.10 timeline are as followed:
>
> * Last posting date: September 15th, 2017
> * Hard code freeze: September 29th, 2017
> * RC1: TBD
> * Release: December 2, 2017
>
> Note that we don't have freeze exception scheme anymore. All patches
> that wish to go into 4.10 must be posted no later than the last posting
> date. All patches posted after that date will be automatically queued
> into next release.
>
> RCs will be arranged immediately after freeze.
>
> We recently introduced a jira instance to track all the tasks (not only big)
> for the project. See: https://xenproject.atlassian.net/projects/XEN/issues.
>
> Most of the tasks tracked by this e-mail also have a corresponding jira task
> referred by XEN-N.
>
> I have started to include the version number of series associated to each
> feature. Can each owner send an update on the version number if the series
> was posted upstream?
>
> = Projects =
>
> == Hypervisor ==
>
> *  Per-cpu tasklet
>   -  XEN-28
>   -  Konrad Rzeszutek Wilk
>
> *  Add support of rcu_idle_{enter,exit}
>   -  XEN-27
>   -  Dario Faggioli
>
> === x86 ===

Could you add the following project to the list?

*  Change xen.efi build and add SHIM_LOCK verification into efi_multiboot2()
  -  Daniel Kiper

This is probably more 4.11 material but let's have it on the 4.10 list too.
Who knows what may happen.

Thanks,

Daniel

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

Re: [Xen-devel] [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: sta...@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

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

Re: [Xen-devel] [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

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

Re: [Xen-devel] [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

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

[Xen-devel] [GRUB2 PATCH RFC 1/1] efi: Add EFI shim lock verifier

[Daniel Kiper]

This is based on git://git.savannah.gnu.org/grub.git phcoder/verifiers branch.

Just an RFC.

TODO:
  - disable the GRUB2 modules load/unload,
  - disable the dangerous modules, e.g. iorw, memrw.

Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com>
---
 grub-core/Makefile.core.def|6 +++
 grub-core/commands/efi/shim_lock.c |  100 
 2 files changed, 106 insertions(+)
 create mode 100644 grub-core/commands/efi/shim_lock.c

diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index 16c4d0e..c38e4a8 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -905,6 +905,12 @@ module = {
 };
 
 module = {
+  name = shim_lock;
+  common = commands/efi/shim_lock.c;
+  enable = x86_64_efi;
+};
+
+module = {
   name = hdparm;
   common = commands/hdparm.c;
   common = lib/hexdump.c;
diff --git a/grub-core/commands/efi/shim_lock.c 
b/grub-core/commands/efi/shim_lock.c
new file mode 100644
index 000..40d2b25
--- /dev/null
+++ b/grub-core/commands/efi/shim_lock.c
@@ -0,0 +1,100 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2017  Free Software Foundation, Inc.
+ *
+ *  GRUB is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  GRUB is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ *  EFI shim lock verifier.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+GRUB_MOD_LICENSE ("GPLv3+");
+
+#define GRUB_EFI_SHIM_LOCK_GUID \
+  { 0x605dab50, 0xe046, 0x4300, \
+{ 0xab, 0xb6, 0x3d, 0xd8, 0x10, 0xdd, 0x8b, 0x23 } \
+  }
+
+struct grub_efi_shim_lock_protocol
+{
+  grub_efi_status_t
+  (*verify) (void *buffer,
+grub_uint32_t size);
+};
+typedef struct grub_efi_shim_lock_protocol grub_efi_shim_lock_protocol_t;
+
+static grub_efi_guid_t shim_lock_guid = GRUB_EFI_SHIM_LOCK_GUID;
+static grub_efi_shim_lock_protocol_t *sl;
+
+static grub_err_t
+shim_lock_init (grub_file_t io __attribute__ ((unused)), enum grub_file_type 
type,
+  void **context __attribute__ ((unused)), enum grub_verify_flags 
*flags)
+{
+  *flags = GRUB_VERIFY_FLAGS_SKIP_VERIFICATION;
+
+  if (!sl)
+return GRUB_ERR_NONE;
+
+  switch (type & GRUB_FILE_TYPE_MASK)
+{
+case GRUB_FILE_TYPE_LINUX_KERNEL:
+case GRUB_FILE_TYPE_MULTIBOOT_KERNEL:
+case GRUB_FILE_TYPE_BSD_KERNEL:
+case GRUB_FILE_TYPE_XNU_KERNEL:
+case GRUB_FILE_TYPE_PLAN9_KERNEL:
+  *flags = GRUB_VERIFY_FLAGS_SINGLE_CHUNK;
+
+default:
+  return GRUB_ERR_NONE;
+}
+}
+
+static grub_err_t
+shim_lock_write (void *context __attribute__ ((unused)), void *buf, 
grub_size_t size)
+{
+  if (sl->verify (buf, size) != GRUB_EFI_SUCCESS)
+return grub_error (GRUB_ERR_BAD_SIGNATURE, N_("bad shim signature"));
+
+  return GRUB_ERR_NONE;
+}
+
+static void
+shim_lock_close (void *context __attribute__ ((unused)))
+{
+}
+
+struct grub_file_verifier shim_lock =
+  {
+.name = "shim_lock",
+.init = shim_lock_init,
+.write = shim_lock_write,
+.close = shim_lock_close
+  };
+
+GRUB_MOD_INIT(shim_lock)
+{
+  sl = grub_efi_locate_protocol (_lock_guid, 0);
+  grub_verifier_register (_lock);
+}
+
+GRUB_MOD_FINI(shim_lock)
+{
+  grub_verifier_unregister (_lock);
+}
-- 
1.7.10.4


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

[Xen-devel] [PATCH RFC 6/7] xen/x86/efi: Verify dom0 kernel with SHIM_LOCK protocol in efi_multiboot2()

[Daniel Kiper]

Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com>
---
 xen/arch/x86/boot/head.S|   20 ++--
 xen/arch/x86/efi/efi-boot.h |   12 +++-
 xen/arch/x86/efi/stub.c |5 -
 3 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
index 90db661..65b3358 100644
--- a/xen/arch/x86/boot/head.S
+++ b/xen/arch/x86/boot/head.S
@@ -383,9 +383,13 @@ __efi64_mb2_start:
 jmp x86_32_switch
 
 .Lefi_multiboot2_proto:
-/* Zero EFI SystemTable and EFI ImageHandle addresses. */
+/*
+ * Zero EFI SystemTable, EFI ImageHandle and
+ * dom0 kernel module struct addresses.
+ */
 xor %esi,%esi
 xor %edi,%edi
+xor %r14d,%r14d
 
 /* Skip Multiboot2 information fixed part. */
 lea (MB2_fixed_sizeof+MULTIBOOT2_TAG_ALIGN-1)(%rbx),%ecx
@@ -423,6 +427,15 @@ __efi64_mb2_start:
 cmove   MB2_efi64_ih(%rcx),%rdi
 je  .Lefi_mb2_next_tag
 
+/* Get dom0 kernel module struct address from Multiboot2 information. 
*/
+cmpl$MULTIBOOT2_TAG_TYPE_MODULE,MB2_tag_type(%rcx)
+jne .Lefi_mb2_end
+
+test%r14d,%r14d
+cmovz   %ecx,%r14d
+jmp .Lefi_mb2_next_tag
+
+.Lefi_mb2_end:
 /* Is it the end of Multiboot2 information? */
 cmpl$MULTIBOOT2_TAG_TYPE_END,MB2_tag_type(%rcx)
 je  .Lrun_bs
@@ -484,9 +497,12 @@ __efi64_mb2_start:
 /* Keep the stack aligned. Do not pop a single item off it. */
 mov (%rsp),%rdi
 
+mov %r14d,%edx
+
 /*
  * efi_multiboot2() is called according to System V AMD64 ABI:
- *   - IN:  %rdi - EFI ImageHandle, %rsi - EFI SystemTable.
+ *   - IN: %rdi - EFI ImageHandle, %rsi - EFI SystemTable,
+ * %rdx - dom0 kernel module struct address.
  */
 callefi_multiboot2
 
diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index bedac5c..6813196 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -3,6 +3,8 @@
  * is intended to be included by common/efi/boot.c _only_, and
  * therefore can define arch specific global variables.
  */
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -47,6 +49,7 @@ extern const struct pe_base_relocs {
 
 static void __init efi_arch_relocate_image(unsigned long delta)
 {
+#if 0
 const struct pe_base_relocs *base_relocs;
 
 for ( base_relocs = __base_relocs_start; base_relocs < __base_relocs_end; )
@@ -95,6 +98,7 @@ static void __init efi_arch_relocate_image(unsigned long 
delta)
 }
 base_relocs = (const void *)(base_relocs->entries + i + (i & 1));
 }
+#endif
 }
 
 extern const s32 __trampoline_rel_start[], __trampoline_rel_stop[];
@@ -669,7 +673,9 @@ static bool __init 
efi_arch_use_config_file(EFI_SYSTEM_TABLE *SystemTable)
 
 static void efi_arch_flush_dcache_area(const void *vaddr, UINTN size) { }
 
-void __init efi_multiboot2(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE 
*SystemTable)
+void __init efi_multiboot2(EFI_HANDLE ImageHandle,
+   EFI_SYSTEM_TABLE *SystemTable,
+   multiboot2_tag_module_t *dom0_kernel)
 {
 EFI_GRAPHICS_OUTPUT_PROTOCOL *gop;
 UINTN cols, gop_mode = ~0, rows;
@@ -687,6 +693,10 @@ void __init efi_multiboot2(EFI_HANDLE ImageHandle, 
EFI_SYSTEM_TABLE *SystemTable
 
 gop = efi_get_gop();
 
+if ( dom0_kernel && dom0_kernel->mod_end > dom0_kernel->mod_start )
+efi_shim_lock((VOID *)(unsigned long)dom0_kernel->mod_start,
+  dom0_kernel->mod_end - dom0_kernel->mod_start);
+
 if ( gop )
 gop_mode = efi_find_gop_mode(gop, 0, 0, 0);
 
diff --git a/xen/arch/x86/efi/stub.c b/xen/arch/x86/efi/stub.c
index 0c481e3..d0cba1d 100644
--- a/xen/arch/x86/efi/stub.c
+++ b/xen/arch/x86/efi/stub.c
@@ -1,7 +1,9 @@
+#include 
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -18,7 +20,8 @@
  */
 
 void __init noreturn efi_multiboot2(EFI_HANDLE ImageHandle,
-EFI_SYSTEM_TABLE *SystemTable)
+EFI_SYSTEM_TABLE *SystemTable,
+multiboot2_tag_module_t *dom0_kernel)
 {
 static const CHAR16 __initconst err[] =
 L"Xen does not have EFI code build in!\r\nSystem halted!\r\n";
-- 
1.7.10.4


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

[Xen-devel] [PATCH RFC 7/7] xen/x86: Build xen.mb.efi directly from xen-syms

[Daniel Kiper]

Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com>
---
 xen/arch/x86/Makefile |1 +
 1 file changed, 1 insertion(+)

diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 93ead6e..e09f5f4 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -95,6 +95,7 @@ syms-warn-dup-$(CONFIG_SUPPRESS_DUPLICATE_SYMBOL_WARNINGS) :=
 $(TARGET): $(TARGET)-syms $(efi-y) boot/mkelf32
./boot/mkelf32 $(notes_phdrs) $(TARGET)-syms $(TARGET) 
$(XEN_IMG_OFFSET) \
   `$(NM) $(TARGET)-syms | sed -ne 's/^\([^ ]*\) . 
__2M_rwdata_end$$/0x\1/p'`
+   $(OBJCOPY) -O binary -S $(TARGET)-syms $(TARGET).mb.efi
 
 ALL_OBJS := $(BASEDIR)/arch/x86/boot/built_in.o 
$(BASEDIR)/arch/x86/efi/built_in.o $(ALL_OBJS)
 
-- 
1.7.10.4


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

[Xen-devel] [PATCH RFC 4/7] xen/x86: Add some addresses to the Multiboot2 header

[Daniel Kiper]

In comparison to ELF the PE format is not supported by the Multiboot2
protocol. So, if we wish to load xen.efi using this protocol we have
to add MULTIBOOT2_HEADER_TAG_ADDRESS and MULTIBOOT2_HEADER_TAG_ENTRY_ADDRESS
tags into Multiboot2 header.

Additionally, put MULTIBOOT2_HEADER_TAG_ENTRY_ADDRESS and
MULTIBOOT2_HEADER_TAG_ENTRY_ADDRESS_EFI64 tags close to each other.

Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com>
---
 xen/arch/x86/boot/head.S |   19 +++
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
index 0c603a5..90db661 100644
--- a/xen/arch/x86/boot/head.S
+++ b/xen/arch/x86/boot/head.S
@@ -237,6 +237,13 @@ multiboot2_header_start:
 /* Align modules at page boundry. */
 mb2ht_init MB2_HT(MODULE_ALIGN), MB2_HT(REQUIRED)
 
+/* The address tag. */
+mb2ht_init MB2_HT(ADDRESS), MB2_HT(REQUIRED), \
+   sym_offs(multiboot2_header_start), /* header_addr */ \
+   sym_offs(start),   /* load_addr */ \
+   sym_offs(__bss_start), /* load_end_addr */ \
+   sym_offs(__2M_rwdata_end)  /* bss_end_addr */
+
 /* Load address preference. */
 mb2ht_init MB2_HT(RELOCATABLE), MB2_HT(OPTIONAL), \
sym_offs(start), /* Min load address. */ \
@@ -244,6 +251,14 @@ multiboot2_header_start:
0x20, /* Load address alignment (2 MiB). */ \
MULTIBOOT2_LOAD_PREFERENCE_HIGH
 
+/* Multiboot2 entry point. */
+mb2ht_init MB2_HT(ENTRY_ADDRESS), MB2_HT(REQUIRED), \
+   sym_offs(__start)
+
+/* EFI64 Multiboot2 entry point. */
+mb2ht_init MB2_HT(ENTRY_ADDRESS_EFI64), MB2_HT(OPTIONAL), \
+   sym_offs(__efi64_mb2_start)
+
 /* Console flags tag. */
 mb2ht_init MB2_HT(CONSOLE_FLAGS), MB2_HT(OPTIONAL), \
MULTIBOOT2_CONSOLE_FLAGS_EGA_TEXT_SUPPORTED
@@ -257,10 +272,6 @@ multiboot2_header_start:
 /* Request that ExitBootServices() not be called. */
 mb2ht_init MB2_HT(EFI_BS), MB2_HT(OPTIONAL)
 
-/* EFI64 Multiboot2 entry point. */
-mb2ht_init MB2_HT(ENTRY_ADDRESS_EFI64), MB2_HT(OPTIONAL), \
-   sym_offs(__efi64_mb2_start)
-
 /* Multiboot2 header end tag. */
 mb2ht_init MB2_HT(END), MB2_HT(REQUIRED)
 .Lmultiboot2_header_end:
-- 
1.7.10.4


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

[Xen-devel] [PATCH RFC 0/7] Change xen.efi build and add SHIM_LOCK verification into efi_multiboot2()

[Daniel Kiper]

Hey,

As in subject... This way we get:
  - one binary which can be loaded by the EFI loader,
Multiboot and Multiboot2 protocols,
  - if we wish, in the future we can drop xen/xen.gz
and build xen.efi only,
  - crash dumps generated by the xen.efi loaded from
the EFI loader can be analyzed by crash tool,
  - simpler code,
  - simpler build,
  - Xen build will no longer depend on ld i386pep support.

This is RFC, so, e.g. xen.mb.efi does not boot if loaded from EFI loader.

TODO:
  - make xen.mb.efi bootable from EFI loader which probably requires
some changes in the code relocating trampoline,
  - drop old PE build code,
  - remove build recipes for old PE code,
  - drop xen ELF build recipes (now or later?),
  - cleanup the code.

Daniel

 xen/Makefile |   14 ---
 xen/arch/x86/Makefile|1 +
 xen/arch/x86/Rules.mk|2 +
 xen/arch/x86/boot/head.S |  197 
+++
 xen/arch/x86/efi/efi-boot.h  |   12 +-
 xen/arch/x86/efi/stub.c  |5 ++-
 xen/arch/x86/xen.lds.S   |   16 ++-
 xen/common/efi/boot.c|   19 ++---
 xen/include/xen/compile.h.in |1 +
 9 files changed, 245 insertions(+), 22 deletions(-)

Daniel Kiper (7):
  xen: Introduce XEN_COMPILE_POSIX_TIME
  xen/x86: Manually build PE header
  xen/x86: Add some addresses to the Multiboot header
  xen/x86: Add some addresses to the Multiboot2 header
  efi: split out efi_shim_lock()
  xen/x86/efi: Verify dom0 kernel with SHIM_LOCK protocol in 
efi_multiboot2()
  xen/x86: Build xen.mb.efi directly from xen-syms


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

[Xen-devel] [PATCH RFC 5/7] efi: split out efi_shim_lock()

[Daniel Kiper]

..which verifies PE signatures with SHIM_LOCK protocol. We want
to re-use this code in subsequent patch in efi_multiboot2().

Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com>
---
 xen/common/efi/boot.c |   19 +--
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 11bdc7a..7db3829 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -124,6 +124,7 @@ static void efi_console_set_mode(void);
 static EFI_GRAPHICS_OUTPUT_PROTOCOL *efi_get_gop(void);
 static UINTN efi_find_gop_mode(EFI_GRAPHICS_OUTPUT_PROTOCOL *gop,
UINTN cols, UINTN rows, UINTN depth);
+static void efi_shim_lock(VOID *Buffer, UINT32 Size);
 static void efi_tables(void);
 static void setup_efi_pci(void);
 static void efi_variables(void);
@@ -797,6 +798,17 @@ static UINTN __init 
efi_find_gop_mode(EFI_GRAPHICS_OUTPUT_PROTOCOL *gop,
 return gop_mode;
 }
 
+static void __init efi_shim_lock(VOID *Buffer, UINT32 Size)
+{
+static EFI_GUID __initdata shim_lock_guid = SHIM_LOCK_PROTOCOL_GUID;
+EFI_SHIM_LOCK_PROTOCOL *shim_lock;
+EFI_STATUS status;
+
+if ( !EFI_ERROR(efi_bs->LocateProtocol(_lock_guid, NULL, (void 
**)_lock)) &&
+ (status = shim_lock->Verify(Buffer, Size)) != EFI_SUCCESS )
+PrintErrMesg(L"Dom0 kernel image could not be verified", status);
+}
+
 static void __init efi_tables(void)
 {
 unsigned int i;
@@ -1062,13 +1074,11 @@ void EFIAPI __init noreturn
 efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
 {
 static EFI_GUID __initdata loaded_image_guid = LOADED_IMAGE_PROTOCOL;
-static EFI_GUID __initdata shim_lock_guid = SHIM_LOCK_PROTOCOL_GUID;
 EFI_LOADED_IMAGE *loaded_image;
 EFI_STATUS status;
 unsigned int i, argc;
 CHAR16 **argv, *file_name, *cfg_file_name = NULL, *options = NULL;
 UINTN gop_mode = ~0;
-EFI_SHIM_LOCK_PROTOCOL *shim_lock;
 EFI_GRAPHICS_OUTPUT_PROTOCOL *gop = NULL;
 union string section = { NULL }, name;
 bool base_video = false;
@@ -1225,10 +1235,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE 
*SystemTable)
 read_file(dir_handle, s2w(), , option_str);
 efi_bs->FreePool(name.w);
 
-if ( !EFI_ERROR(efi_bs->LocateProtocol(_lock_guid, NULL,
-(void **)_lock)) &&
- (status = shim_lock->Verify(kernel.ptr, kernel.size)) != 
EFI_SUCCESS )
-PrintErrMesg(L"Dom0 kernel image could not be verified", status);
+   efi_shim_lock(kernel.ptr, kernel.size);
 
 name.s = get_value(, section.s, "ramdisk");
 if ( name.s )
-- 
1.7.10.4


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

[Xen-devel] [PATCH RFC 2/7] xen/x86: Manually build PE header

[Daniel Kiper]

This is the first step to get:
  - one binary which can be loaded by the EFI loader,
Multiboot and Multiboot2 protocols,
  - if we wish, in the future we can drop xen/xen.gz
and build xen.efi only,
  - crash dumps generated by the xen.efi loaded from
the EFI loader can be analyzed by crash tool,
  - simpler code,
  - simpler build,
  - Xen build will no longer depend on ld i386pep support.

Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com>
---
 xen/arch/x86/Rules.mk|2 +
 xen/arch/x86/boot/head.S |  145 ++
 xen/arch/x86/xen.lds.S   |   16 -
 3 files changed, 162 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk
index 568657e..b501c88 100644
--- a/xen/arch/x86/Rules.mk
+++ b/xen/arch/x86/Rules.mk
@@ -7,6 +7,8 @@ CFLAGS += -I$(BASEDIR)/include
 CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-generic
 CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-default
 CFLAGS += -DXEN_IMG_OFFSET=$(XEN_IMG_OFFSET)
+CFLAGS += -DXEN_LOAD_ALIGN=XEN_IMG_OFFSET
+CFLAGS += -DXEN_FILE_ALIGN=PAGE_SIZE
 CFLAGS += '-D__OBJECT_LABEL__=$(subst /,$$,$(subst -,_,$(subst 
$(BASEDIR)/,,$(CURDIR))/$@))'
 
 # Prevent floating-point variables from creeping into Xen.
diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
index fd6fc33..28bbc04 100644
--- a/xen/arch/x86/boot/head.S
+++ b/xen/arch/x86/boot/head.S
@@ -1,3 +1,4 @@
+#include 
 #include 
 #include 
 #include 
@@ -44,6 +45,150 @@
 .Lmb2ht_init_end\@:
 .endm
 
+.section .efi.pe.header, "a", @progbits
+
+ENTRY(efi_pe_head)
+/*
+ * Legacy EXE header.
+ *
+ * Most of it is copied from binutils package, version 2.28,
+ * include/coff/pe.h:struct external_PEI_filehdr and
+ * bfd/peXXigen.c:_bfd_XXi_only_swap_filehdr_out().
+ *
+ * Page is equal 512 bytes here.
+ * Paragraph is equal 16 bytes here.
+ */
+.short  0x5a4d /* EXE magic number. */
+.short  0x90   /* Bytes on last page of 
file. */
+.short  0x3/* Pages in file. */
+.short  0  /* Relocations. */
+.short  0x4/* Size of header in 
paragraphs. */
+.short  0  /* Minimum extra paragraphs 
needed. */
+.short  0x /* Maximum extra paragraphs 
needed. */
+.short  0  /* Initial (relative) SS 
value. */
+.short  0xb8   /* Initial SP value. */
+.short  0  /* Checksum. */
+.short  0  /* Initial IP value. */
+.short  0  /* Initial (relative) CS 
value. */
+.short  0x40   /* File address of 
relocation table. */
+.short  0  /* Overlay number. */
+.fill   4, 2, 0/* Reserved words. */
+.short  0  /* OEM identifier. */
+.short  0  /* OEM information. */
+.fill   10, 2, 0   /* Reserved words. */
+.long   pe_header - efi_pe_head/* File address of the PE 
header. */
+
+/*
+ * DOS message.
+ *
+ * It is copied from binutils package, version 2.28,
+ * include/coff/pe.h:struct external_PEI_filehdr and
+ * bfd/peXXigen.c:_bfd_XXi_only_swap_filehdr_out().
+ */
+.long   0x0eba1f0e
+.long   0xcd09b400
+.long   0x4c01b821
+.long   0x685421cd
+.long   0x70207369
+.long   0x72676f72
+.long   0x63206d61
+.long   0x6f6e6e61
+.long   0x65622074
+.long   0x6e757220
+.long   0x206e6920
+.long   0x20534f44
+.long   0x65646f6d
+.long   0x0a0d0d2e
+.long   0x24
+.long   0
+
+/*
+ * PE/COFF header.
+ *
+ * The PE/COFF format is defined by Microsoft, and is available from
+ * http://www.microsoft.com/whdc/system/platform/firmware/PECOFF.mspx
+ * 
+ * Some ideas are taken from Linux kernel and Xen ARM64.
+ */
+
+pe_header:
+.ascii  "PE\0\0"   /* PE signature. */
+.short  0x8664 /* Machine: 
IMAGE_FILE_MACHINE_AMD64. */
+.short  1  /* NumberOfSections. */
+.long   XEN_COMPILE_POSIX_TIME /* TimeDateStamp. */
+.long   0  /* PointerToSymbolTable. */
+.long   0  /* NumberOfSymbols. */
+   

[Xen-devel] [PATCH RFC 3/7] xen/x86: Add some addresses to the Multiboot header

[Daniel Kiper]

In comparison to ELF the PE format is not supported by the Multiboot
protocol. So, if we wish to load xen.efi using this protocol we have
to put header_addr, load_addr, load_end_addr, bss_end_addr and
entry_addr data into Multiboot header.

Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com>
---
 xen/arch/x86/boot/head.S |   13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
index 28bbc04..0c603a5 100644
--- a/xen/arch/x86/boot/head.S
+++ b/xen/arch/x86/boot/head.S
@@ -195,13 +195,24 @@ ENTRY(start)
 .align 4
 multiboot1_header_start:   /*** MULTIBOOT1 HEADER /
 #define MULTIBOOT_HEADER_FLAGS (MULTIBOOT_HEADER_MODS_ALIGNED | \
-MULTIBOOT_HEADER_WANT_MEMORY)
+MULTIBOOT_HEADER_WANT_MEMORY | \
+MULTIBOOT_HEADER_HAS_ADDR)
 /* Magic number indicating a Multiboot header. */
 .long   MULTIBOOT_HEADER_MAGIC
 /* Flags to bootloader (see Multiboot spec). */
 .long   MULTIBOOT_HEADER_FLAGS
 /* Checksum: must be the negated sum of the first two fields. */
 .long   -(MULTIBOOT_HEADER_MAGIC + MULTIBOOT_HEADER_FLAGS)
+/* header_addr */
+.long   sym_offs(multiboot1_header_start)
+/* load_addr */
+.long   sym_offs(start)
+/* load_end_addr */
+.long   sym_offs(__bss_start)
+/* bss_end_addr */
+.long   sym_offs(__2M_rwdata_end)
+/* entry_addr */
+.long   sym_offs(__start)
 multiboot1_header_end:
 
 /*** MULTIBOOT2 HEADER /
-- 
1.7.10.4


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

[Xen-devel] [PATCH RFC 1/7] xen: Introduce XEN_COMPILE_POSIX_TIME

[Daniel Kiper]

We need the POSIX time to properly fill the TimeDateStamp field in the PE 
header.

Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com>
---
 xen/Makefile |   14 --
 xen/include/xen/compile.h.in |1 +
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/xen/Makefile b/xen/Makefile
index f6a6bc2..2424690 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -6,12 +6,13 @@ export XEN_EXTRAVERSION ?= -unstable$(XEN_VENDORVERSION)
 export XEN_FULLVERSION   = $(XEN_VERSION).$(XEN_SUBVERSION)$(XEN_EXTRAVERSION)
 -include xen-version
 
-export XEN_WHOAMI  ?= $(USER)
-export XEN_DOMAIN  ?= $(shell ([ -x /bin/dnsdomainname ] && 
/bin/dnsdomainname) || ([ -x /bin/domainname ] && /bin/domainname || echo 
[unknown]))
-export XEN_BUILD_DATE  ?= $(shell LC_ALL=C date)
-export XEN_BUILD_TIME  ?= $(shell LC_ALL=C date +%T)
-export XEN_BUILD_HOST  ?= $(shell hostname)
-export XEN_CONFIG_EXPERT ?= n
+export XEN_WHOAMI  ?= $(USER)
+export XEN_DOMAIN  ?= $(shell ([ -x /bin/dnsdomainname ] && 
/bin/dnsdomainname) || ([ -x /bin/domainname ] && /bin/domainname || echo 
[unknown]))
+export XEN_BUILD_DATE  ?= $(shell LC_ALL=C date)
+export XEN_BUILD_TIME  ?= $(shell LC_ALL=C date +%T)
+export XEN_BUILD_POSIX_TIME?= $(shell LC_ALL=C date +%s)
+export XEN_BUILD_HOST  ?= $(shell hostname)
+export XEN_CONFIG_EXPERT   ?= n
 
 export BASEDIR := $(CURDIR)
 export XEN_ROOT := $(BASEDIR)/..
@@ -164,6 +165,7 @@ delete-unfresh-files:
 include/xen/compile.h: include/xen/compile.h.in .banner
@sed -e 's/@@date@@/$(XEN_BUILD_DATE)/g' \
-e 's/@@time@@/$(XEN_BUILD_TIME)/g' \
+   -e 's/@@posix_time@@/$(XEN_BUILD_POSIX_TIME)/g' \
-e 's/@@whoami@@/$(XEN_WHOAMI)/g' \
-e 's/@@domain@@/$(XEN_DOMAIN)/g' \
-e 's/@@hostname@@/$(XEN_BUILD_HOST)/g' \
diff --git a/xen/include/xen/compile.h.in b/xen/include/xen/compile.h.in
index 440ecb2..b2ae6f9 100644
--- a/xen/include/xen/compile.h.in
+++ b/xen/include/xen/compile.h.in
@@ -1,5 +1,6 @@
 #define XEN_COMPILE_DATE   "@@date@@"
 #define XEN_COMPILE_TIME   "@@time@@"
+#define XEN_COMPILE_POSIX_TIME @@posix_time@@
 #define XEN_COMPILE_BY "@@whoami@@"
 #define XEN_COMPILE_DOMAIN "@@domain@@"
 #define XEN_COMPILE_HOST   "@@hostname@@"
-- 
1.7.10.4


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

Re: [Xen-devel] Optimising the DevSummit schedule on July 11

[Daniel Kiper]

On Tue, Jul 04, 2017 at 09:01:27AM +0100, Roger Pau Monne wrote:
> On Mon, Jul 03, 2017 at 11:37:29AM +0100, Lars Kurth wrote:
> > Folks, (committers and speakers/moderators CC'ed)
> >
> > I have a few extra sessions from Jan which came in today. Most of Tuesday 
> > in x86 stuff, so there is no space. I merged one of my session with a 
> > proposal from Jan, but it seems to me that the July 11 schedule would work 
> > better the following way (see picture)
> >
> >
> >
> > The only problem we will have is that at least either Stefano or Julien 
> > need to be part of the Graphics session, as well as Paul Durrant and some 
> > EPAM/Intel folks dealing with Graphics and co-processor sharing. But we 
> > should have enough key people in the Community Problem session.
> >
> > If I don't hear substantial objections by tomorrow: I will go for it.
>
> Hello,
>
> Can the two EFI sessions be merged? I would like to attend
> some EFI related stuff since at some point I will have to implement it
> for FreeBSD (and I cannot really see that much difference between the
> two sessions, but I really know very little about EFI), but right now
> at least one of them is clashing with my PVH toolstack session.

I am OK with rescheduling my sessions in other way. Though I am not
convinced about merging them. What will happen with second topic if
we consume whole time for the first one?

Daniel

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

Re: [Xen-devel] OOPS new Xen 4.9.0_08 / kernel 4.12.0 Dom0 crash @ domain_crash_sync called from entry.S: fault at ffff82d080342328 entry.o#create_bounce_frame+0x135/0x14d

[Daniel Kiper]

On Wed, Jul 05, 2017 at 10:27:19AM -0700, PGNet Dev wrote:
> On 7/5/17 12:58 AM, Jan Beulich wrote:
> >So there are two problems here: One is the fact that the kernel
> >really should put an Invalid Opcode exception handler in place
> >before intentionally raising any such exceptions (which WARN()
> >and WARN_ON() do). The other is that Linux commit 636259880a
> >("efi: Add support for seeding the RNG from a UEFI config table")
> >failed to also update arch/x86/xen/efi.c, so the caller
> >(efi_config_parse_tables()) tries to map a gigantic amount of
> >memory, based on the value it found at NULL (which it then uses
> >as the size to map). Luckily the fix for it is in Linus'es tree already -
> >commit 6c64447ec5 ("x86/xen/efi: Initialize only the EFI struct
> >members used by Xen"). It's marked for stable backport, but it
> >fails to mention the commit it fixes.

It fails because I did not know about the issue in advance.
I posted the fix as an aticipation of potential problems.

> iiuc, sounds like
>
> -- wait for in-the-pipeline @kernel fixes to simply propagate

Or backport 6c64447 (x86/xen/efi: Initialize only the EFI struct members
used by Xen) and 457ea3f (efi: Process the MEMATTR table only if EFI_MEMMAP is
enabled). Latter is an extra fix in this case but worth backporting too.

> -- nothing to be done @xen

I hope so.

Daniel

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

Re: [Xen-devel] Optimising the DevSummit schedule on July 11

[Daniel Kiper]

On Mon, Jul 03, 2017 at 11:37:29AM +0100, Lars Kurth wrote:
> Folks, (committers and speakers/moderators CC'ed)
>
> I have a few extra sessions from Jan which came in today. Most of Tuesday
> in x86 stuff, so there is no space. I merged one of my session with a proposal
> from Jan, but it seems to me that the July 11 schedule would work better the
> following way (see picture)
>
> The only problem we will have is that at least either Stefano or Julien need
> to be part of the Graphics session, as well as Paul Durrant and some 
> EPAM/Intel
> folks dealing with Graphics and co-processor sharing. But we should have 
> enough
> key people in the Community Problem session.

I am OK with this change.

Daniel

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

[Xen-devel] [PATCH v2 0/2] xen/efi: Fixes

[Daniel Kiper]

Hey,

Two small fixes (v2, minor cleanup) for Xen dom0 running on x86_64 EFI 
platforms.

I am CC-ing stable maintainers because similar stuff is needed for various
stable kernels too. Unfortunately, almost every version needs a bit different
set of fixes. So, please treat this email more as head up than real set of
patches for your kernel. If you wish to get Xen EFI stuff fixed just drop me
a line. Then I will prepare set of patches for your kernel (if needed).

Ard, Andrew, Ingo, thank you for looking at the patches.

Daniel

 arch/x86/xen/efi.c |   45 -
 drivers/firmware/efi/efi.c |3 ++-
 2 files changed, 14 insertions(+), 34 deletions(-)

Daniel Kiper (2):
  efi: Process MEMATTR table only if EFI_MEMMAP
  x86/xen/efi: Init only efi struct members used by Xen


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

[Xen-devel] [PATCH v2 1/2] efi: Process MEMATTR table only if EFI_MEMMAP

[Daniel Kiper]

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

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

Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com>
Reviewed-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
---
 drivers/firmware/efi/efi.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index b372aad..045d6d3 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -528,7 +528,8 @@ int __init efi_config_parse_tables(void *config_tables, int 
count, int sz,
}
}
 
-   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) {
-- 
1.7.10.4


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

[Xen-devel] [PATCH v2 2/2] x86/xen/efi: Init only efi struct members used by Xen

[Daniel Kiper]

Current approach, wholesale efi struct initialization from efi_xen, is not
good. 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. As I saw it happened
a few times until now. So, let's initialize only efi struct members used by
Xen to avoid such issues in the future.

Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com>
Acked-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
--
Align assignments to increase readability. Suggested by Ingo Molnar.
---
 arch/x86/xen/efi.c |   45 -
 1 file changed, 12 insertions(+), 33 deletions(-)

diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c
index 30bb2e8..a18703b 100644
--- a/arch/x86/xen/efi.c
+++ b/arch/x86/xen/efi.c
@@ -54,38 +54,6 @@
.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_efi_probe(void)
 
/* 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;
-- 
1.7.10.4


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

Re: [Xen-devel] [PATCH 2/2] x86/xen/efi: Init only efi struct members used by Xen

[Daniel Kiper]

On Wed, Jun 21, 2017 at 11:24:06AM +0200, Ingo Molnar wrote:
>
> * Daniel Kiper <daniel.ki...@oracle.com> wrote:
>
> > -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_efi_probe(void)
> >
> > /* 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;
>
> This is a step back stylistically, as you lost the nice vertical tabulation 
> of the
> original initializer ...

If you wish and others do not object I can realign it back.

Daniel

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

Re: [Xen-devel] [PATCH 2/2] x86/xen/efi: Init only efi struct members used by Xen

[Daniel Kiper]

On Wed, Jun 21, 2017 at 09:10:51AM +0100, Andrew Cooper wrote:
> On 20/06/2017 21:14, Daniel Kiper wrote:
> > Current approach, wholesale efi struct initialization from efi_xen, is not
> > good. 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. As I saw it 
> > happened
> > a few times until now. So, let's initialize only efi struct members used by
> > Xen to avoid such issues in the future.
> >
> > Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com>
> > ---
> >  arch/x86/xen/efi.c |   45 -
> >  1 file changed, 12 insertions(+), 33 deletions(-)
> >
> > diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c
> > index 30bb2e8..01b9faf 100644
> > --- a/arch/x86/xen/efi.c
> > +++ b/arch/x86/xen/efi.c
> > @@ -54,38 +54,6 @@
> > .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_efi_probe(void)
> >
> > /* 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;
>
> This presumably means that the system default values are already present
> in efi at the point that we overwrite some Xen specifics?

More or less.

> If so, surely you need to retain the clobbering of set_virtual_address_map ?

Nope, by default efi.set_virtual_address_map is NULL (please take a look
at efi struct initialization in drivers/firmware/efi/efi.c). And it is
not touched if efi_enabled(EFI_PARAVIRT).

Daniel

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

[Xen-devel] [PATCH 2/2] x86/xen/efi: Init only efi struct members used by Xen

[Daniel Kiper]

Current approach, wholesale efi struct initialization from efi_xen, is not
good. 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. As I saw it happened
a few times until now. So, let's initialize only efi struct members used by
Xen to avoid such issues in the future.

Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com>
---
 arch/x86/xen/efi.c |   45 -
 1 file changed, 12 insertions(+), 33 deletions(-)

diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c
index 30bb2e8..01b9faf 100644
--- a/arch/x86/xen/efi.c
+++ b/arch/x86/xen/efi.c
@@ -54,38 +54,6 @@
.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_efi_probe(void)
 
/* 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;
-- 
1.7.10.4


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

[Xen-devel] [PATCH 1/2] efi: Process MEMATTR table only if EFI_MEMMAP

[Daniel Kiper]

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

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

Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com>
---
 drivers/firmware/efi/efi.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index b372aad..045d6d3 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -528,7 +528,8 @@ int __init efi_config_parse_tables(void *config_tables, int 
count, int sz,
}
}
 
-   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) {
-- 
1.7.10.4


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

[Xen-devel] [PATCH 0/2] xen/efi: Fixes

[Daniel Kiper]

Hey,

Two small fixes for Xen dom0 running on x86_64 EFI platforms.

I am CC-ing stable maintainers because similar stuff is needed for various
stable kernels too. Unfortunately, almost every version needs a bit different
set of fixes. So, please treat this email more as head up than real set of
patches for your kernel. If you wish to get Xen EFI stuff fixed just drop me
a line. Then I will prepare set of patches for your kernel (if needed).

Daniel

 arch/x86/xen/efi.c |   45 -
 drivers/firmware/efi/efi.c |3 ++-
 2 files changed, 14 insertions(+), 34 deletions(-)

Daniel Kiper (2):
  efi: Process MEMATTR table only if EFI_MEMMAP
  x86/xen/efi: Init only efi struct members used by Xen


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

Re: [Xen-devel] [PATCH v2] x86/boot: Fix the boot time relocation calculations

[Daniel Kiper]

On Mon, Jun 12, 2017 at 11:45:43AM +0100, Andrew Cooper wrote:
> c/s b28044226e1 "x86: make Xen early boot code relocatable" introduces
>
> mov $sym_offs(__image_base__),%esi
>
> to the legacy boot path.  However, this is by definition 0, which means the
> boot code only functions correctly when Xen is loaded at its preferred
> physical address (2M at the time of writing).
>
> Xen does cope if loaded at an alternative physical address, if the
> MULTIBOOT2_TAG_TYPE_LOAD_BASE_ADDR tag is filled in properly.  While recent
> versions of Grub do fill this in appropriately, tboot does not.  (In fact,
> tboot loads Xen at the preferred address, but claims a load address of 8M.)
>
> Both Multiboot 1 and 2 specify the execution environment as being flat.  As a
> result, Xen needs no help calculating the proper load address.
>
> However, Multiboot specifies %esp as undefined.  Experimentally, using the
> entry %esp is fine, but this is certainly no guarantee.  Use a temporary stack
> in the first page of RAM, which is one of the safest areas to clobber.
>
> Calculate the load address from %eip alone, and ignore
> MULTIBOOT2_TAG_TYPE_LOAD_BASE_ADDR entirely.  This fixes legacy boot under
> various versions of tboot.
>
> Finally, set up the stack as soon as possible, which means the BIOS path has a
> usable stack for the entirety of its duration.  Use the full available stack
> size, rather than limiting to an arbitrary 1k.  One side effect is that the
> MB2/EFI path continues to use the EFI stack until the trampoline is entered.
>
> Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>
> Tested-by: Sergey Dyasli <sergey.dya...@citrix.com>

Reviewed-by: Daniel Kiper <daniel.ki...@oracle.com>

Daniel

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

Re: [Xen-devel] [PATCH for-4.9] x86/boot: Fix the boot time relocation calculations

[Daniel Kiper]

On Fri, Jun 02, 2017 at 05:57:46PM +0100, Andrew Cooper wrote:
> c/s b28044226e1 "x86: make Xen early boot code relocatable" introduces
>
> mov $sym_offs(__image_base__),%esi
>
> to the legacy boot path.  However, this is by definition 0, which means the
> boot code only functions correctly when Xen is loaded at its preferred
> physical address (2M at the time of writing).
>
> Xen does cope if loaded at an alternative physical address, if the
> MULTIBOOT2_TAG_TYPE_LOAD_BASE_ADDR tag is filled in properly.  While recent
> versions of Grub do fill this in appropriately, tboot does not.  (In fact,
> tboot loads Xen at the preferred address, but claims a load address of 8M.)

Does it use Multiboot or Multiboot2 protocol? Anyway, I think that this
should be fixed in tboot.

> Both multiboot 1 and 2 specify the execution environment as being flat.  As a
> result, Xen needs no help calculating the proper load address.
>
> Calculate the load address from %eip alone, and ignore
> MULTIBOOT2_TAG_TYPE_LOAD_BASE_ADDR entirely.  This fixes legacy boot under
> various versions of tboot.
>
> Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>
> Tested-by: Sergey Dyasli <sergey.dya...@citrix.com>
> ---
> CC: Jan Beulich <jbeul...@suse.com>
> CC: Julien Grall <julien.gr...@arm.com>
> CC: Daniel Kiper <daniel.ki...@oracle.com>
> CC: Doug Goldstein <car...@cardoe.com>
> CC: Sergey Dyasli <sergey.dya...@citrix.com>
>
> This is a regression introduced in Xen 4.9, and should therefore be fixed.
> ---
>  xen/arch/x86/boot/head.S | 15 ---
>  1 file changed, 4 insertions(+), 11 deletions(-)
>
> diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
> index 5e84e42..df28b09 100644
> --- a/xen/arch/x86/boot/head.S
> +++ b/xen/arch/x86/boot/head.S
> @@ -377,8 +377,10 @@ __start:
>  cld
>  cli
>
> -/* Load default Xen image load base address. */
> -mov $sym_offs(__image_base__),%esi
> +/* Calculate the load base address. */
> +call1f
> +1:  pop %esi
> +sub $sym_offs(1b), %esi

I have a problem with this. I know that this should be done in that way. Even
I considered that during my work on Multiboot2 stuff. However Multiboot and
Multiboot2 specs clearly say: The OS image must create its own stack as soon
as it needs one. So, we cannot assume here that %ss:%esp points to valid stack
and do such things. Even if it works with this and that right now. So, I would
prefer to fix it in more reliable way right now even if it means more code here
and there.

Daniel

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

Re: [Xen-devel] [Crash-utility] [PATCH] xen: Add support for domU with Linux kernel 3.19 and newer

[Daniel Kiper]

On Wed, May 24, 2017 at 11:56:28AM -0400, Dave Anderson wrote:
> - Original Message -
> > crash patch c3413456599161cabc4e910a0ae91dfe5eec3c21 (xen: Add support for
> > dom0 with Linux kernel 3.19 and newer) from Daniel made crash utility
> > support xen dom0 vmcores after linux kernel commit
> > 054954eb051f35e74b75a566a96fe756015352c8 (xen: switch to linear virtual
> > mapped sparse p2m list).
> >
> > This patch can be deemed as a subsequent and make this utility support Xen
> > PV domU dumpfiles again.
> >
> > Basically speaking, readmem() can't be used to read xen_p2m_addr associate
> > memory directly during m2p translation. It introduces infinite recursion.
> > Following call sequence shows the scenario, it comes from a section of
> > backtrace with only kvaddr, machine addr and mfn left as parameter:
> >
> > module_init()
> >
> > /* The first readmem() from module_init(). */
> > readmem(addr=0xa02fe4a0)
> >
> > /* readmem() needs physical address, so calls kvtop(). */
> > kvtop(kvaddr=0xa02fe4a0)
> > x86_64_kvtop(kvaddr=a02fe4a0)
> >
> > /* Calculate physical address by traversing page tables. */
> > x86_64_kvtop_xen_wpt(kvaddr=0xa02fe4a0)
> >
> > /*
> >  * x86_64_kvtop_xen_wpt() is going to traverse the page table to
> >  * get the physical address for 0xa02fe4a0. So, at first it
> >  * is needed to translate the pgd from machine address to physical
> >  * address. So invoke xen_m2p() here to do the translation. 0x58687f000
> >  * is the pgd machine address in x86_64_kvtop_xen_wpt() and is needed
> >  * to be translated to its physical address.
> >  */
> > xen_m2p(machine=0x58687f000)
> > __xen_m2p(machine=0x58687f000, mfn=0x58687f)
> >
> > /*
> >  * __xen_m2p() is going to search mfn 0x58687f in p2m VMA which starts
> >  * at VMA 0xc91cf000. It compares every mfn stored in it with
> >  * 0x58687f. Once it's proved 0x58687f is one mfn in the p2m, its offset
> >  * will be used to calculate the pfn.
> >  *
> >  * readmem() is invoked by __xen_m2p() to read the page from VMA
> >  * 0xc91cf000 here.
> >  */
> > readmem(addr=0xc91cf000)
> >
> > /*
> >  * readmem() needs physical address of 0xc91cf000 to make the
> >  * reading done. So it invokes kvtop() to get the physical address.
> >  */
> > kvtop(kvaddr=0xc91cf000)
> > x86_64_kvtop(kvaddr=0xc91cf000)
> >
> > /* It needs to calculate physical address by traversing page tables. */
> > x86_64_kvtop_xen_wpt(kvaddr=0xc91cf000)
> >
> > /*
> >  * 0x581b7e000 is the machine address of pgd need to be translated here.
> >  * The mfn is calculated in this way at x86_64_kvtop_xen_wpt():
> >  *
> >  * pml4 = ((ulong *)machdep->machspec->pml4) + pml4_index(kvaddr);
> >  * pgd_paddr = (*pml4) & PHYSICAL_PAGE_MASK;
> >  *
> >  * The kvaddr 0xc91cf000 here is quite different from the one
> >  * above, so the machine address of pgd is not the same one. And this
> >  * pgd is the one we use to access the VMA of p2m table.
> >  */
> > xen_m2p(machine=0x581b7e000)
> > __xen_m2p(machine=0x581b7e000, mfn=0x581b7e)
> >
> > /*
> >  * Looking for mfn 0x581b7e in the range of p2m page which starts at
> >  * VMA 0xc91f5000.
> >  */
> > readmem(addr=0xc91f5000)
> >
> > /* Need physical address of VMA 0xc91f5000 as same reason above. */
> > kvtop(kvaddr=0xc91f5000)
> > x86_64_kvtop(kvaddr=0xc91f5000)
> >
> > /* Need to traverse page tables to calculate physical address for it. */
> > x86_64_kvtop_xen_wpt(kvaddr=0xc91f5000)
> >
> > /*
> >  * Unfortunately, machine address 0x581b7e000 have to be translated again.
> >  * Endless loop starts from here.
> >  */
> > xen_m2p(machine=0x581b7e000)
> > __xen_m2p(machine=0x581b7e000, mfn=0x581b7e)
> > readmem(addr=0xc91f5000)
> >
> > Fortunately, PV domU p2m mapping is also stored at xd->xfd + 
> > xch_index_offset
> > and organized as struct xen_dumpcore_p2m. We have a chance to read the p2m
> > stuff directly from there, and then we avoid the loop above.
> >
> > So, this patch implements a special reading function read_xc_p2m() to 
> > extract
> > the mfns from xd->xfd + xch_index_offset. This function does not need to 
> > read
> > mfns from p2m VMA like readmem() does, so, we avoid the endless loop 
> > introduced
> > by the address translation.
> >
> > Signed-off-by: Honglei Wang <honglei.w...@oracle.com>
> > Reviewed-by: Daniel Kiper <daniel.ki...@oracle.com>
>
> Queued for crash-7.2.0:
>
>   
> https://github.com/crash-utility/crash/commit/5c52842a58a2602dba81de71831af98b2b53c6e0

Wow, Dave, you are fast! Thanks a lot!

Honglei, congrats! Thanks for doing the work!

Daniel

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

Re: [Xen-devel] [PATCH v8 0/3] arm64, xen: add xen_boot support into grub-mkconfig

[Daniel Kiper]

On Mon, May 15, 2017 at 03:46:55PM +0200, Daniel Kiper wrote:
> Hi Julien,
>
> On Mon, May 15, 2017 at 02:43:28PM +0100, Julien Grall wrote:
> > Hi Daniel,
> >
> > On 15/05/17 14:38, Daniel Kiper wrote:
> > >On Sun, May 14, 2017 at 03:43:44PM +0800, fu@linaro.org wrote:
> > >>From: Fu Wei <fu@linaro.org>
> > >>
> > >>This patchset add xen_boot support into grub-mkconfig for
> > >>generating xen boot entrances automatically
> > >>
> > >>Also update the docs/grub.texi for new xen_boot commands.
> > >
> > >LGTM, if there are no objections I will commit it at the end
> > >of this week or the beginning of next one.
> >
> > Thank you!
> >
> > Can you also please commit patch [1] which has been sitting on the grub
> > ML for more than a year? This is preventing to boot Xen ARM with GRUB.
> >
> > Cheers,
> >
> > [1] https://lists.gnu.org/archive/html/grub-devel/2016-02/msg00205.html
>
> Will do with this patch series.

Committed...

Daniel

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

Re: [Xen-devel] [Xen-users] UEFI Secure Boot Xen 4.9

[Daniel Kiper]

On Mon, May 15, 2017 at 07:09:54PM +, Bill Jacobs (billjac) wrote:
> > -Original Message-
> > From: Daniel Kiper [mailto:daniel.ki...@oracle.com]
> > Sent: Monday, May 15, 2017 6:13 AM
> > To: Bill Jacobs (billjac) <bill...@cisco.com>; george.dun...@citrix.com
> > Cc: xen-devel@lists.xen.org; xen-us...@lists.xen.org
> > Subject: Re: [Xen-users] UEFI Secure Boot Xen 4.9
> >
> > Hey,
> >
> > CC-ing Xen-devel to spread some knowledge about the issue.
> >
> > On Mon, May 15, 2017 at 10:42:23AM +0100, George Dunlap wrote:
> > > On Wed, May 10, 2017 at 11:36 PM, Bill Jacobs (billjac)
> > > <bill...@cisco.com> wrote:
> > > > Hi all
> > > >
> > > > I gather that with 4.9, UEFI secure boot of Xen should be possible.
> > > >
> > > > Is this true?
> > > >
> > > > If so, what are the options for utilizing UEFI secure boot? Do I
> > > > need a MSFT-signed shim or grub? Any special changes required for
> > > > Xen kernel
> > > > (signing?) or has that been done?
> > >
> > > Bill,
> > >
> > > I guess in part it depends on what you mean by "utilizing UEFI secure
> > > boot".  If you simply want to boot an unsigned Xen on a UEFI system
> > > with SecureBoot enabled, then grub would probably work.  If you want
> > > to actually do the full SecureBoot thing -- where you have grub check
> > > Xen's signature and that of the kernel and initrd, you probably need a
> > > bit more.
> > >
> > > Daniel,
> > >
> > > Is there any good documentation on this?  The Xen EFI guide
> > > (https://wiki.xenproject.org/wiki/Xen_EFI) mentions the shim, but
> > > doesn't go into detail about how to sign a binary 
> >
> > Unfortunately I do not know anything like that. As you said in general shim 
> > is
> > supported. Sadly, it works only if you load xen.efi directly from EFI.
> > __Upstream__ GRUB2 has not have support for shim yet. I am working on it
> > (shim support via GRUB2 requires also some changes in Xen). I hope that I 
> > will
> > have something which works before Xen conf in Budapest.
> >
> > If you wish to use shim with xen.efi then you have to sign xen.efi and 
> > vmlinux
> > with your key using sbsign or pesign. The process works in the same way 
> > like in
> > case vmlinux alone. Of course you have to install your public key into MOK
> > before enabling secure boot.
> >
> > Daniel
>
> Yes, there are options in how this is achievable, and the solutions may be 
> different.
>
> We are targeting a secure boot chain from UEFI fw to .ko, using same signing.
> In our case would skip shim and reduce attack surface, but it appears that 
> the mechanisms
> 'out there' for passing pub key (cert) from UEFI db to Linux chainring 
> require shim to do
> the work. Is that accurate? Does it have to be the case? I don't see why.

AIUI, if EFI secure boot is enabled then EFI verifies signatures of every
loaded/executed PE file. Unfortunately, you are not able to use secure boot
protocol directly to verify yourself PE's loaded from your app. So, this is
one of reasons why shim was introduced. It exposes protocol which can be
used by you to do verification.

> For us, ideal case is :
> UEFI fw -> (signed)GRUB2.efi->Multiboot2->Xen(signed .ko)

AFAICT, it is not possible. We should do following thing:

  UEFI -> shim -> GRUB2 -> Multiboot2 -> Xen/Linux/etc.

UEFI will verify shim secure boot signature then shim will verify GRUB2
signature then GRUB2 will verify (with shim protocol) Xen signature and
finally Xen will verify (with shim protocol) Linux kernel signature. Then
your kernel can verify modules using whatever you want.

> I would be happy to work to help achieve this.

There is a chance that I will have something very raw at the beginning
of June. If you wish to do tests drop me a line.

Daniel

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

Re: [Xen-devel] [PATCH v8 0/3] arm64, xen: add xen_boot support into grub-mkconfig

[Daniel Kiper]

Hi Julien,

On Mon, May 15, 2017 at 02:43:28PM +0100, Julien Grall wrote:
> Hi Daniel,
>
> On 15/05/17 14:38, Daniel Kiper wrote:
> >On Sun, May 14, 2017 at 03:43:44PM +0800, fu@linaro.org wrote:
> >>From: Fu Wei <fu@linaro.org>
> >>
> >>This patchset add xen_boot support into grub-mkconfig for
> >>generating xen boot entrances automatically
> >>
> >>Also update the docs/grub.texi for new xen_boot commands.
> >
> >LGTM, if there are no objections I will commit it at the end
> >of this week or the beginning of next one.
>
> Thank you!
>
> Can you also please commit patch [1] which has been sitting on the grub
> ML for more than a year? This is preventing to boot Xen ARM with GRUB.
>
> Cheers,
>
> [1] https://lists.gnu.org/archive/html/grub-devel/2016-02/msg00205.html

Will do with this patch series.

Daniel

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

Re: [Xen-devel] [PATCH v8 0/3] arm64, xen: add xen_boot support into grub-mkconfig

[Daniel Kiper]

On Sun, May 14, 2017 at 03:43:44PM +0800, fu@linaro.org wrote:
> From: Fu Wei 
>
> This patchset add xen_boot support into grub-mkconfig for
> generating xen boot entrances automatically
>
> Also update the docs/grub.texi for new xen_boot commands.

LGTM, if there are no objections I will commit it at the end
of this week or the beginning of next one.

Daniel

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

Re: [Xen-devel] [Xen-users] UEFI Secure Boot Xen 4.9

[Daniel Kiper]

Hey,

CC-ing Xen-devel to spread some knowledge about the issue.

On Mon, May 15, 2017 at 10:42:23AM +0100, George Dunlap wrote:
> On Wed, May 10, 2017 at 11:36 PM, Bill Jacobs (billjac)
>  wrote:
> > Hi all
> >
> > I gather that with 4.9, UEFI secure boot of Xen should be possible.
> >
> > Is this true?
> >
> > If so, what are the options for utilizing UEFI secure boot? Do I need a
> > MSFT-signed shim or grub? Any special changes required for Xen kernel
> > (signing?) or has that been done?
>
> Bill,
>
> I guess in part it depends on what you mean by "utilizing UEFI secure
> boot".  If you simply want to boot an unsigned Xen on a UEFI system
> with SecureBoot enabled, then grub would probably work.  If you want
> to actually do the full SecureBoot thing -- where you have grub check
> Xen's signature and that of the kernel and initrd, you probably need a
> bit more.
>
> Daniel,
>
> Is there any good documentation on this?  The Xen EFI guide
> (https://wiki.xenproject.org/wiki/Xen_EFI) mentions the shim, but
> doesn't go into detail about how to sign a binary 

Unfortunately I do not know anything like that. As you said in general
shim is supported. Sadly, it works only if you load xen.efi directly from
EFI. __Upstream__ GRUB2 has not have support for shim yet. I am working
on it (shim support via GRUB2 requires also some changes in Xen). I hope
that I will have something which works before Xen conf in Budapest.

If you wish to use shim with xen.efi then you have to sign xen.efi and
vmlinux with your key using sbsign or pesign. The process works in the same
way like in case vmlinux alone. Of course you have to install your public
key into MOK before enabling secure boot.

Daniel

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

Re: [Xen-devel] [PATCH v7 0/3] arm64, xen: add xen_boot support into grup-mkconfig

[Daniel Kiper]

Hey,

On Tue, May 02, 2017 at 03:06:24PM +0800, fu@linaro.org wrote:
> From: Fu Wei 
>
> This patchset add xen_boot support into grup-mkconfig for
> generating xen boot entrances automatically
>
> Also update the docs/grub.texi for new xen_boot commands.

Slowly recovering after long weekend in Poland.
I will take a look at this probably next week.

Daniel

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

Re: [Xen-devel] [PATCH v4 2/2] kexec: remove spinlock now that all KEXEC hypercall ops are protected at the top-level

[Daniel Kiper]

On Thu, Apr 20, 2017 at 01:14:30PM +0100, Andrew Cooper wrote:
> On 20/04/17 11:46, Jan Beulich wrote:
> >>>> On 19.04.17 at 23:01, <eric.devol...@oracle.com> wrote:
> >> The spinlock in kexec_swap_images() was removed as
> >> this function is only reachable on the kexec hypercall, which is
> >> now protected at the top-level in do_kexec_op_internal(),
> >> thus the local spinlock is no longer necessary.
> >>
> >> Per recommendation from Jan Beulich and Andrew Cooper, I left
> >> an ASSERT in place of the spin_lock().
> >>
> >> Signed-off-by: Eric DeVolder <eric.devol...@oracle.com>
> >> Reviewed-by: Bhavesh Davda <bhavesh.da...@oracle.com>
> >> Reviewed-by: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
> > Reviewed-by: Jan Beulich <jbeul...@suse.com>
>
> Reviewed-by: Andrew Cooper <andrew.coop...@citrix.com>

Reviewed-by: Daniel Kiper <daniel.ki...@oracle.com>

Daniel

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

Re: [Xen-devel] [PATCH v3 1/2] kexec: use hypercall_create_continuation to protect KEXEC ops

[Daniel Kiper]

On Thu, Apr 20, 2017 at 03:34:21AM -0600, Jan Beulich wrote:
> >>> On 19.04.17 at 19:16,  wrote:
> > On Wed, Apr 19, 2017 at 10:19:44AM -0600, Jan Beulich wrote:
> >> >>> On 19.04.17 at 17:54,  wrote:
> >> > On Wed, Apr 19, 2017 at 10:47:15AM -0500, Eric DeVolder wrote:
> >> >> @@ -1193,6 +1194,9 @@ static int do_kexec_op_internal(unsigned long op,
> >> >>  if ( ret )
> >> >>  return ret;
> >> >>
> >> >> +if ( test_and_set_bit(KEXEC_FLAG_IN_HYPERCALL, _flags) )
> >> >> +return hypercall_create_continuation(__HYPERVISOR_kexec_op, 
> >> >> "lh", op, uarg);
> >> >> +
> >> >
> >> > I would suggest here:
> >> >ASSERT(test_bit(KEXEC_FLAG_IN_HYPERCALL, _flags));
> >>
> >> You're kidding? The flag was set just in the line above. Or do you
> >> really mean we need to consider test_and_set_bit() not doing what
> >> it is supposed to do?
> >
> > Yep, it looks ridiculous. However, ASSERT() in kexec_swap_images() looks 
> > almost
> > the same for me. So, TBH, I still do not understand need for it at all. 
> > Could
> > you enlighten me?
>
> Can't be that difficult to understand: There was a lock there before,
> and the addition of the ASSERT() could help document that the
> serialization requirements aren't being broken. I'm not saying there

Ahhh... OK, so, you treat this more as a comment than anything else.
So, why not use regular comment here then? Hmmm... Do you treat an
ASSERT() here as kind of fuse too?

> might not be other places to _also_ add ASSERT()s, but not in _that
> other_ patch.

OK, so, I would put ASSERT() at least at the beginning of kexec_load()
and kexec_unload() (I am not sure about others). However, then ASSERT()
is not needed in kexec_swap_images(). Though if you wish to leave it
I will not object any longer.

Daniel

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

Re: [Xen-devel] [PATCH] arm64: xen: Implement EFI reset_system callback

[Daniel Kiper]

On Wed, Apr 19, 2017 at 08:37:38PM +0100, Matt Fleming wrote:
> On Wed, 19 Apr, at 09:29:06PM, Daniel Kiper wrote:
> > On Tue, Apr 18, 2017 at 02:46:50PM +0100, Matt Fleming wrote:
> > > On Thu, 06 Apr, at 04:55:11PM, Mark Rutland wrote:
> > > >
> > > > Please, let's keep the Xen knowledge constrained to the Xen EFI wrapper,
> > > > rather than spreading it further.
> > > >
> > > > IMO, given reset_system is a *mandatory* function, the Xen wrapper
> > > > should provide an implementation.
> > > >
> > > > I don't see why you can't implement a wrapper that calls the usual Xen
> > > > poweroff/reset functions.
> > >
> > > I realise I'm making a sweeping generalisation, but adding
> > > EFI_PARAVIRT is almost always the wrong thing to do.
> >
> > Why?
>
> Because it makes paravirt a special case, and there's usually very
> little reason to make it special in the EFI code. Special-casing means
> more branches, more code paths, a bigger testing matrix and more
> complex code.
>
> EFI_PARAVIRT does have its uses, like for those scenarios where we
> don't have a table of function pointers that can be overidden for
> paravirt.
>
> But we do have such a table for ->reset_system().

This is more or less what I expected. Thanks a lot for explanation.

Daniel

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

Re: [Xen-devel] [PATCH] arm64: xen: Implement EFI reset_system callback

[Daniel Kiper]

On Tue, Apr 18, 2017 at 02:46:50PM +0100, Matt Fleming wrote:
> On Thu, 06 Apr, at 04:55:11PM, Mark Rutland wrote:
> >
> > Please, let's keep the Xen knowledge constrained to the Xen EFI wrapper,
> > rather than spreading it further.
> >
> > IMO, given reset_system is a *mandatory* function, the Xen wrapper
> > should provide an implementation.
> >
> > I don't see why you can't implement a wrapper that calls the usual Xen
> > poweroff/reset functions.
>
> I realise I'm making a sweeping generalisation, but adding
> EFI_PARAVIRT is almost always the wrong thing to do.

Why?

> I'd much prefer to see Mark's idea implemented.

If you wish I do not object.

Daniel

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

Re: [Xen-devel] [PATCH v3 1/2] kexec: use hypercall_create_continuation to protect KEXEC ops

[Daniel Kiper]

On Wed, Apr 19, 2017 at 10:19:44AM -0600, Jan Beulich wrote:
> >>> On 19.04.17 at 17:54,  wrote:
> > On Wed, Apr 19, 2017 at 10:47:15AM -0500, Eric DeVolder wrote:
> >> @@ -1193,6 +1194,9 @@ static int do_kexec_op_internal(unsigned long op,
> >>  if ( ret )
> >>  return ret;
> >>
> >> +if ( test_and_set_bit(KEXEC_FLAG_IN_HYPERCALL, _flags) )
> >> +return hypercall_create_continuation(__HYPERVISOR_kexec_op, "lh", 
> >> op, uarg);
> >> +
> >
> > I would suggest here:
> >ASSERT(test_bit(KEXEC_FLAG_IN_HYPERCALL, _flags));
>
> You're kidding? The flag was set just in the line above. Or do you
> really mean we need to consider test_and_set_bit() not doing what
> it is supposed to do?

Yep, it looks ridiculous. However, ASSERT() in kexec_swap_images() looks almost
the same for me. So, TBH, I still do not understand need for it at all. Could
you enlighten me?

If we really need it I would put it at the beginning of every function called
from switch() in do_kexec_op_internal(). Just in case. Though I still do not
see the point for it. At least in that form.

Daniel

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

Re: [Xen-devel] [PATCH v3 2/2] kexec: remove spinlock now that all KEXEC hypercall ops are protected at the top-level

[Daniel Kiper]

On Wed, Apr 19, 2017 at 10:47:16AM -0500, Eric DeVolder wrote:
> The spinlock in kexec_swap_images() was removed as
> this function is only reachable on the kexec hypercall, which is
> now protected at the top-level in do_kexec_op_internal(),
> thus the local spinlock is no longer necessary.
>
> Per recommendation from Jan Beulich and Andrew Cooper, I left
> an ASSERT in place of the spin_lock().
>
> Signed-off-by: Eric DeVolder 
> Reviewed-by: Bhavesh Davda 
> Reviewed-by: Konrad Rzeszutek Wilk 
> ---
> v3:
>  - Incorporated feedback from Jan Beulich and Andrew Cooper
>to leave an ASSERT where spin_lock() once was.
>
> v2: 04/17/2017
>  - Patch titled 'kexec: use hypercall_create_continuation to protect KEXEC 
> ops'
>  - Separated removal of spinlock in kexec_swap_images() into its own patch.
>
> v1: 04/10/2017
>  - Patch titled 'kexec: Add spinlock for the whole hypercall'
>  - Removal of spinlock in kexec_swap_images() was part of other patch.
> ---
>  xen/common/kexec.c | 5 +
>  1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/xen/common/kexec.c b/xen/common/kexec.c
> index 253c204..af32e58 100644
> --- a/xen/common/kexec.c
> +++ b/xen/common/kexec.c
> @@ -820,7 +820,6 @@ static int kexec_exec(XEN_GUEST_HANDLE_PARAM(void) uarg)
>  static int kexec_swap_images(int type, struct kexec_image *new,
>   struct kexec_image **old)
>  {
> -static DEFINE_SPINLOCK(kexec_lock);
>  int base, bit, pos;
>  int new_slot, old_slot;
>
> @@ -832,7 +831,7 @@ static int kexec_swap_images(int type, struct kexec_image 
> *new,
>  if ( kexec_load_get_bits(type, , ) )
>  return -EINVAL;
>
> -spin_lock(_lock);
> +ASSERT( !test_bit(KEXEC_FLAG_IN_HYPERCALL, _flags) );

...and drop ASSERT() here.

Daniel

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

Re: [Xen-devel] [PATCH v3 1/2] kexec: use hypercall_create_continuation to protect KEXEC ops

[Daniel Kiper]

On Wed, Apr 19, 2017 at 10:47:15AM -0500, Eric DeVolder wrote:
> When we concurrently try to unload and load crash
> images we eventually get:
>
>  Xen call trace:
> [] machine_kexec_add_page+0x3a0/0x3fa
> [] machine_kexec_load+0xdb/0x107
> [] kexec.c#kexec_load_slot+0x11/0x42
> [] kexec.c#kexec_load+0x119/0x150
> [] kexec.c#do_kexec_op_internal+0xab/0xcf
> [] do_kexec_op+0xe/0x1e
> [] pv_hypercall+0x20a/0x44a
> [] cpufreq.c#test_all_events+0/0x30
>
>  Pagetable walk from 820040088320:
>   L4[0x104] = 0002979d1063 
>   L3[0x001] = 0002979d0063 
>   L2[0x000] = 0002979c7063 
>   L1[0x088] = 80037a91ede97063 
>
> The interesting thing is that the page bits (063) look legit.
>
> The operation on which we blow up is us trying to write
> in the L1 and finding that the L2 entry points to some
> bizzare MFN. It stinks of a race, and it looks like
> the issue is due to no concurrency locks when dealing
> with the crash kernel space.
>
> Specifically we concurrently call kimage_alloc_crash_control_page
> which iterates over the kexec_crash_area.start -> kexec_crash_area.size
> and once found:
>
>   if ( page )
>   {
>   image->next_crash_page = hole_end;
>   clear_domain_page(_mfn(page_to_mfn(page)));
>   }
>
> clears. Since the parameters of what MFN to use are provided
> by the callers (and the area to search is bounded) the the 'page'
> is probably the same. So #1 we concurrently clear the
> 'control_code_page'.
>
> The next step is us passing this 'control_code_page' to
> machine_kexec_add_page. This function requires the MFNs:
> page_to_maddr(image->control_code_page).
>
> And this would always return the same virtual address, as
> the MFN of the control_code_page is inside of the
> kexec_crash_area.start -> kexec_crash_area.size area.
>
> Then machine_kexec_add_page updates the L1 .. which can be done
> concurrently and on subsequent calls we mangle it up.
>
> This is all a theory at this time, but testing reveals
> that adding the hypercall_create_continuation() at the
> kexec hypercall fixes the crash.
>
> NOTE: This patch follows 5c5216 (kexec: clear kexec_image slot
> when unloading kexec image) to prevent crashes during
> simultaneous load/unloads.
>
> NOTE: Consideration was given to using the existing flag
> KEXEC_FLAG_IN_PROGRESS to denote a kexec hypercall in
> progress. This, however, overloads the original intent of
> the flag which is to denote that we are about-to/have made
> the jump to the crash path. The overloading would lead to
> failures in existing checks on this flag as the flag would
> always be set at the top level in do_kexec_op_internal().
> For this reason, the new flag KEXEC_FLAG_HC_IN_PROGRESS
> was introduced.
>
> While at it, fixed the #define mismatched spacing
>
> Signed-off-by: Eric DeVolder <eric.devol...@oracle.com>
> Reviewed-by: Bhavesh Davda <bhavesh.da...@oracle.com>
> Reviewed-by: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
> Reviewed-by: Jan Beulich <jbeul...@suse.com>
> Reviewed-by: Andrew Cooper <andrew.coop...@citrix.com>
> Reviewed-by: Daniel Kiper <daniel.ki...@oracle.com>
> ---
> v3:
>  - Incorporated feedback from Jan Beulich to change the
>name of the new flag to KEXEC_FLAG_IN_HYPERCALL
>
> v2: 04/17/2017
>  - Patch titled 'kexec: use hypercall_create_continuation to protect KEXEC 
> ops'
>  - Jan Beulich directed me to use a continuation method instead
>of spinlock.
>  - Incorporated feedback from Daniel Kiper <daniel.ki...@oracle.com>
>  - Incorporated feedback from Konrad Wilk <konrad.w...@oracle.com>
>
> v1: 04/10/2017
>  - Patch titled 'kexec: Add spinlock for the whole hypercall'
>  - Used spinlock in do_kexec_op_internal()
> ---
>  xen/common/kexec.c | 12 +---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/xen/common/kexec.c b/xen/common/kexec.c
> index 072cc8e..253c204 100644
> --- a/xen/common/kexec.c
> +++ b/xen/common/kexec.c
> @@ -50,9 +50,10 @@ static cpumask_t crash_saved_cpus;
>
>  static struct kexec_image *kexec_image[KEXEC_IMAGE_NR];
>
> -#define KEXEC_FLAG_DEFAULT_POS   (KEXEC_IMAGE_NR + 0)
> -#define KEXEC_FLAG_CRASH_POS (KEXEC_IMAGE_NR + 1)
> -#define KEXEC_FLAG_IN_PROGRESS   (KEXEC_IMAGE_NR + 2)
> +#define KEXEC_FLAG_DEFAULT_POS  (KEXEC_IMAGE_NR + 0)
> +#define KEXEC_FLAG_CRASH_POS(KEXEC_IMAGE_NR + 1)
> +#define KEXEC_FLAG_IN_PROGRESS  (KEXEC_IMAGE_NR + 2)
> +#define KEXEC_FLAG_IN_HYPERCALL (KEXEC_IMAGE_NR + 3)

I do not think that you need change all of this right now.
Just a

Re: [Xen-devel] [PATCH v2 1/2] kexec: use hypercall_create_continuation to protect KEXEC ops

[Daniel Kiper]

On Wed, Apr 19, 2017 at 12:48:56PM +0100, Andrew Cooper wrote:
> On 19/04/17 12:00, Daniel Kiper wrote:
> > On Tue, Apr 18, 2017 at 04:48:06AM -0600, Jan Beulich wrote:
> >>>>> On 17.04.17 at 21:09, <eric.devol...@oracle.com> wrote:
> >>> --- a/xen/common/kexec.c
> >>> +++ b/xen/common/kexec.c
> >>> @@ -50,9 +50,10 @@ static cpumask_t crash_saved_cpus;
> >>>
> >>>  static struct kexec_image *kexec_image[KEXEC_IMAGE_NR];
> >>>
> >>> -#define KEXEC_FLAG_DEFAULT_POS   (KEXEC_IMAGE_NR + 0)
> >>> -#define KEXEC_FLAG_CRASH_POS (KEXEC_IMAGE_NR + 1)
> >>> -#define KEXEC_FLAG_IN_PROGRESS   (KEXEC_IMAGE_NR + 2)
> >>> +#define KEXEC_FLAG_DEFAULT_POS(KEXEC_IMAGE_NR + 0)
> >>> +#define KEXEC_FLAG_CRASH_POS  (KEXEC_IMAGE_NR + 1)
> >>> +#define KEXEC_FLAG_IN_PROGRESS(KEXEC_IMAGE_NR + 2)
> >>> +#define KEXEC_FLAG_HC_IN_PROGRESS (KEXEC_IMAGE_NR + 3)
> >> Perhaps KEXEC_FLAG_IN_HYPERCALL? Other than that (and this
> > Make sense for me.
>
> Agreed.  (I can fix on commit).
>
> >
> >> clearly is subject to Andrew's opinion)
> >> Reviewed-by: Jan Beulich <jbeul...@suse.com>
> > Otherwise Reviewed-by: Daniel Kiper <daniel.ki...@oracle.com>
>
> Reviewed-by: Andrew Cooper <andrew.coop...@citrix.com>
>
> CC'ing Julien.  This is clearly a good bugfix for 4.9

Yep and I think that it is stable material too.

Daniel

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

Re: [Xen-devel] [PATCH v2 2/2] kexec: remove spinlock now that all KEXEC hypercall ops are protected at the top-level

[Daniel Kiper]

On Wed, Apr 19, 2017 at 05:20:50AM -0600, Jan Beulich wrote:
> >>> On 19.04.17 at 12:56,  wrote:
> > On Tue, Apr 18, 2017 at 04:49:48AM -0600, Jan Beulich wrote:
> >> >>> On 17.04.17 at 21:09,  wrote:
> >> > The spinlock in kexec_swap_images() was removed as
> >> > this function is only reachable on the kexec hypercall, which is
> >> > now protected at the top-level in do_kexec_op_internal(),
> >> > thus the local spinlock is no longer necessary.
> >>
> >> But perhaps leave an ASSERT() there, making sure the in-hypercall
> >> flag is set?
> >
> > I am not sure why but if at all I think that we should also consider
> > other key kexec functions then. Or put ASSERT() into do_kexec_op_internal()
> > just before "switch ( op )".
>
> The point of my placement suggestion was that the ASSERT()
> effectively replaces the lock acquire - the places you name
> didn't previously require any synchronization.

After the first patch of this series kexec_swap_images() cannot be
started twice in parallel. So, I do not see the point of ASSERT() here.
Or let's say we wish to have it to double check that "the in-hypercall
flag is set". AIUI, it is your original idea. However, then I think that
we should have an ASSERT() at least in kexec_load_slot() because parallel
loads make issues too. And we can go higher to feel more safe. That is
why I suggested do_kexec_op_internal() as the final resting place for
an ASSERT(). Simply it looks to me the safest approach. Am I missing
something?

Daniel

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

Re: [Xen-devel] [PATCH v2 1/2] kexec: use hypercall_create_continuation to protect KEXEC ops

[Daniel Kiper]

On Tue, Apr 18, 2017 at 04:48:06AM -0600, Jan Beulich wrote:
> >>> On 17.04.17 at 21:09, <eric.devol...@oracle.com> wrote:
> > --- a/xen/common/kexec.c
> > +++ b/xen/common/kexec.c
> > @@ -50,9 +50,10 @@ static cpumask_t crash_saved_cpus;
> >
> >  static struct kexec_image *kexec_image[KEXEC_IMAGE_NR];
> >
> > -#define KEXEC_FLAG_DEFAULT_POS   (KEXEC_IMAGE_NR + 0)
> > -#define KEXEC_FLAG_CRASH_POS (KEXEC_IMAGE_NR + 1)
> > -#define KEXEC_FLAG_IN_PROGRESS   (KEXEC_IMAGE_NR + 2)
> > +#define KEXEC_FLAG_DEFAULT_POS(KEXEC_IMAGE_NR + 0)
> > +#define KEXEC_FLAG_CRASH_POS  (KEXEC_IMAGE_NR + 1)
> > +#define KEXEC_FLAG_IN_PROGRESS(KEXEC_IMAGE_NR + 2)
> > +#define KEXEC_FLAG_HC_IN_PROGRESS (KEXEC_IMAGE_NR + 3)
>
> Perhaps KEXEC_FLAG_IN_HYPERCALL? Other than that (and this

Make sense for me.

> clearly is subject to Andrew's opinion)
> Reviewed-by: Jan Beulich <jbeul...@suse.com>

Otherwise Reviewed-by: Daniel Kiper <daniel.ki...@oracle.com>

Daniel

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

Re: [Xen-devel] [PATCH v2 2/2] kexec: remove spinlock now that all KEXEC hypercall ops are protected at the top-level

[Daniel Kiper]

On Tue, Apr 18, 2017 at 04:49:48AM -0600, Jan Beulich wrote:
> >>> On 17.04.17 at 21:09,  wrote:
> > The spinlock in kexec_swap_images() was removed as
> > this function is only reachable on the kexec hypercall, which is
> > now protected at the top-level in do_kexec_op_internal(),
> > thus the local spinlock is no longer necessary.
>
> But perhaps leave an ASSERT() there, making sure the in-hypercall
> flag is set?

I am not sure why but if at all I think that we should also consider
other key kexec functions then. Or put ASSERT() into do_kexec_op_internal()
just before "switch ( op )".

Daniel

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

Re: [Xen-devel] [PATCH v2] xen, kdump: handle pv domain in paddr_vmcoreinfo_note()

[Daniel Kiper]

On Fri, Apr 14, 2017 at 06:53:36PM +0200, Petr Tesarik wrote:
> On Tue, 11 Apr 2017 19:20:08 +0200
> Daniel Kiper <daniel.ki...@oracle.com> wrote:
> > On Tue, Apr 11, 2017 at 04:59:16PM +0200, Petr Tesarik wrote:
> >[...]
> > > Tested-by: Petr Tesarik <ptesa...@suse.com>
> > >
> > > I copied the complete /proc/vmcore to a directory on disk. Exactly
> > > as expected, crash works both without the patch and with the patch, as
> > > it does not use VMCOREINFO at all (instead, crash obtains the
> > > information from kernel debuginfo directly).
> >
> > Thanks for doing the tests. I suppose that you have tested HVM guests.
>
> Not really. I crashed Dom0, which is in turn sent to the hypervisor, so
> the result is a complete host dump, including Xen hypervisor data and
> all domains.

OK.

> > IIRC, PV guests are not supported by crash right now due to p2m VMA
> > mapping. At least it was an issue some time ago. Is it still valid?
>
> Yes, this is correct. I tested this behaviour a few weeks ago.

Thanks for update.

> > Anyway, one guy in Oracle works on fix for that issue and I do review.
> > We are going to post it in 2-3 weeks.
>
> All right. FYI I do not plan to put much effort into it, as my focus has

OK.

> shifted towards libkdumpfile (https://github.com/ptesarik/libkdumpfile),
> and this library can open PV guest dump files without any issues.

Great! AIUI, it reminds my idea to make such think. However, I have not
have time to make it happen. Is it based on makedumpfile or written from
scratch? Do you plan support for Linux kernel dumps and/or Xen ones?

Daniel

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

Re: [Xen-devel] EFI + tboot + Xen

[Daniel Kiper]

On Fri, Apr 14, 2017 at 04:17:54PM +0100, Andrew Cooper wrote:
> On 14/04/2017 15:54, Daniel Kiper wrote:
> > Hey,
> >
> > Has anybody tried to run EFI + tboot + Xen?
> > I have a feeling that it does not work because
> > tboot shuts down EFI boot services. However,
> > even if it works then efibootmgr is unusable
> > due to lack of EFI runtime services. Do we care?
> > Is it possible to make it work with full blown
> > EFI infrastructure available for Xen?
>
> Judging by
> http://hg.code.sf.net/p/tboot/code/file/9352e6391332/tboot/common/boot.S#l83
> it will be grub exiting boot services.  tboot needs rather more
> multiboot2 knowledge before it could participate in a hand-off to Xen
> while keeping boot services active.

Sure, it is not a problem. However, I was told that it was (not) done
deliberately because we cannot trust EFI due to lack of its measurement.
I am not sure it is true or not. I though that somebody played with tboot
and Xen and has some knowledge in that area. Anyway, I will investigate
this further. However, any knowledge sharing is greatly appreciated.

Daniel

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

[Xen-devel] EFI + tboot + Xen

[Daniel Kiper]

Hey,

Has anybody tried to run EFI + tboot + Xen?
I have a feeling that it does not work because
tboot shuts down EFI boot services. However,
even if it works then efibootmgr is unusable
due to lack of EFI runtime services. Do we care?
Is it possible to make it work with full blown
EFI infrastructure available for Xen?

Daniel

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

Re: [Xen-devel] [PATCH v16 7/9] x86: make Xen early boot code relocatable

[Daniel Kiper]

On Thu, Apr 13, 2017 at 09:44:17PM -0400, Konrad Rzeszutek Wilk wrote:
> On Thu, Apr 13, 2017 at 04:11:25PM +0200, Daniel Kiper wrote:
> > On Fri, Apr 07, 2017 at 05:23:33AM -0600, Jan Beulich wrote:
> > > >>> On 21.02.17 at 20:19, <daniel.ki...@oracle.com> wrote:
> > > > Every multiboot protocol (regardless of version) compatible image must
> > > > specify its load address (in ELF or multiboot header). Multiboot 
> > > > protocol
> > > > compatible loader have to load image at specified address. However, 
> > > > there
> > > > is no guarantee that the requested memory region (in case of Xen it 
> > > > starts
> > > > at 2 MiB and ends at ~5 MiB) where image should be loaded initially is 
> > > > a RAM
> > > > and it is free (legacy BIOS platforms are merciful for Xen but I found 
> > > > at
> > > > least one EFI platform on which Xen load address conflicts with EFI boot
> > > > services; it is Dell PowerEdge R820 with latest firmware). To cope with 
> > > > that
> > > > problem we must make Xen early boot code relocatable and help boot 
> > > > loader to
> > > > relocate image in proper way by suggesting, not requesting specific load
> > > > addresses as it is right now, allowed address ranges. This patch does
> > > > former.
> > > > It does not add multiboot2 protocol interface which is done in "x86: add
> > > > multiboot2 protocol support for relocatable images" patch.
> > > >
> > > > This patch changes following things:
> > > >   - %esi register is used as a storage for Xen image load base address;
> > > > it is mostly unused in early boot code and preserved during C 
> > > > functions
> > > > calls in 32-bit mode,
> > > >   - %fs is used as base for Xen data relative addressing in 32-bit code
> > > > if it is possible; %esi is used for that thing during error printing
> > > > because it is not always possible to properly and efficiently
> > > > initialize %fs.
> > > >
> > > > Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com>
> > >
> > > Reviewed-by: Jan Beulich <jbeul...@suse.com>
> >
> > It looks that everything passed through test gate and landed in master.
> > So, this way we have full multiboot2 support in Xen. This means that
> > you can boot Xen using GRUB2 on EFI platforms.
> >
> > I would like to thank everybody who helped me to make it happen.
> > Especially Jan who patiently reviewed whole series many times
> > and replied for my stupid questions.
>
> 

Cheers!

> Yey! Congrats!

Thank you.

Daniel

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

Re: [Xen-devel] [PATCH v16 7/9] x86: make Xen early boot code relocatable

[Daniel Kiper]

On Thu, Apr 13, 2017 at 02:43:22PM -0500, Doug Goldstein wrote:
> On 4/13/17 9:11 AM, Daniel Kiper wrote:
> > On Fri, Apr 07, 2017 at 05:23:33AM -0600, Jan Beulich wrote:
> >>>>> On 21.02.17 at 20:19, <daniel.ki...@oracle.com> wrote:
> >>> Every multiboot protocol (regardless of version) compatible image must
> >>> specify its load address (in ELF or multiboot header). Multiboot protocol
> >>> compatible loader have to load image at specified address. However, there
> >>> is no guarantee that the requested memory region (in case of Xen it starts
> >>> at 2 MiB and ends at ~5 MiB) where image should be loaded initially is a 
> >>> RAM
> >>> and it is free (legacy BIOS platforms are merciful for Xen but I found at
> >>> least one EFI platform on which Xen load address conflicts with EFI boot
> >>> services; it is Dell PowerEdge R820 with latest firmware). To cope with 
> >>> that
> >>> problem we must make Xen early boot code relocatable and help boot loader 
> >>> to
> >>> relocate image in proper way by suggesting, not requesting specific load
> >>> addresses as it is right now, allowed address ranges. This patch does
> >>> former.
> >>> It does not add multiboot2 protocol interface which is done in "x86: add
> >>> multiboot2 protocol support for relocatable images" patch.
> >>>
> >>> This patch changes following things:
> >>>   - %esi register is used as a storage for Xen image load base address;
> >>> it is mostly unused in early boot code and preserved during C 
> >>> functions
> >>>     calls in 32-bit mode,
> >>>   - %fs is used as base for Xen data relative addressing in 32-bit code
> >>> if it is possible; %esi is used for that thing during error printing
> >>> because it is not always possible to properly and efficiently
> >>> initialize %fs.
> >>>
> >>> Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com>
> >>
> >> Reviewed-by: Jan Beulich <jbeul...@suse.com>
> >
> > It looks that everything passed through test gate and landed in master.
> > So, this way we have full multiboot2 support in Xen. This means that
> > you can boot Xen using GRUB2 on EFI platforms.
> >
> > I would like to thank everybody who helped me to make it happen.
> > Especially Jan who patiently reviewed whole series many times
> > and replied for my stupid questions.
> >
> > Daniel
>
> Congrats Daniel.

Thank you.

Daniel

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

Re: [Xen-devel] [PATCH v16 7/9] x86: make Xen early boot code relocatable

[Daniel Kiper]

On Fri, Apr 07, 2017 at 05:23:33AM -0600, Jan Beulich wrote:
> >>> On 21.02.17 at 20:19, <daniel.ki...@oracle.com> wrote:
> > Every multiboot protocol (regardless of version) compatible image must
> > specify its load address (in ELF or multiboot header). Multiboot protocol
> > compatible loader have to load image at specified address. However, there
> > is no guarantee that the requested memory region (in case of Xen it starts
> > at 2 MiB and ends at ~5 MiB) where image should be loaded initially is a RAM
> > and it is free (legacy BIOS platforms are merciful for Xen but I found at
> > least one EFI platform on which Xen load address conflicts with EFI boot
> > services; it is Dell PowerEdge R820 with latest firmware). To cope with that
> > problem we must make Xen early boot code relocatable and help boot loader to
> > relocate image in proper way by suggesting, not requesting specific load
> > addresses as it is right now, allowed address ranges. This patch does
> > former.
> > It does not add multiboot2 protocol interface which is done in "x86: add
> > multiboot2 protocol support for relocatable images" patch.
> >
> > This patch changes following things:
> >   - %esi register is used as a storage for Xen image load base address;
> > it is mostly unused in early boot code and preserved during C functions
> > calls in 32-bit mode,
> >   - %fs is used as base for Xen data relative addressing in 32-bit code
> > if it is possible; %esi is used for that thing during error printing
> > because it is not always possible to properly and efficiently
> > initialize %fs.
> >
> > Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com>
>
> Reviewed-by: Jan Beulich <jbeul...@suse.com>

It looks that everything passed through test gate and landed in master.
So, this way we have full multiboot2 support in Xen. This means that
you can boot Xen using GRUB2 on EFI platforms.

I would like to thank everybody who helped me to make it happen.
Especially Jan who patiently reviewed whole series many times
and replied for my stupid questions.

Daniel

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

Re: [Xen-devel] [PATCH v2] xen, kdump: handle pv domain in paddr_vmcoreinfo_note()

[Daniel Kiper]

On Tue, Apr 11, 2017 at 04:59:16PM +0200, Petr Tesarik wrote:
> On Tue, 11 Apr 2017 15:00:58 +0200
> Daniel Kiper <daniel.ki...@oracle.com> wrote:
>
> > On Tue, Apr 11, 2017 at 02:45:43PM +0200, Juergen Gross wrote:
> > > On 03/04/17 14:42, Daniel Kiper wrote:
> > > > On Fri, Mar 31, 2017 at 12:14:38PM +0200, Juergen Gross wrote:
> > > >> For kdump to work correctly it needs the physical address of
> > > >> vmcoreinfo_note. When running as dom0 this means the virtual address
> > > >> has to be translated to the related machine address.
> > > >>
> > > >> paddr_vmcoreinfo_note() is meant to do the translation via
> > > >> __pa_symbol() only, but being attributed "weak" it can be replaced
> > > >> easily in Xen case.
> > > >>
> > > >> Signed-off-by: Juergen Gross <jgr...@suse.com>
> > > >
> > > > Have you tested this patch with latest crash tool? Do dom0 and Xen
> > > > hypervisor analysis work without any issue (at least basic commands
> > > > like dmesg, bt, ps, etc.)? If yes for both you can add:
> > > >
> > > > Reviewed-by: Daniel Kiper <daniel.ki...@oracle.com>
> > >
> > > So can I add your R-b: now?
> >
> > My R-b is still valid. Though, let's wait for Petr's Tested-by. He
> > did makedumpfile tests but I asked him to do crash tool tests too.
> > I think it is important.
>
> Tested-by: Petr Tesarik <ptesa...@suse.com>
>
> I copied the complete /proc/vmcore to a directory on disk. Exactly
> as expected, crash works both without the patch and with the patch, as
> it does not use VMCOREINFO at all (instead, crash obtains the
> information from kernel debuginfo directly).

Thanks for doing the tests. I suppose that you have tested HVM guests.
IIRC, PV guests are not supported by crash right now due to p2m VMA
mapping. At least it was an issue some time ago. Is it still valid?
Anyway, one guy in Oracle works on fix for that issue and I do review.
We are going to post it in 2-3 weeks.

Juergen, I do not have any objections any longer. You can go ahead.
Ahhh... I see. You have posted v3. Good!

Daniel

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

Re: [Xen-devel] [PATCH v2] xen, kdump: handle pv domain in paddr_vmcoreinfo_note()

[Daniel Kiper]

On Tue, Apr 11, 2017 at 02:45:43PM +0200, Juergen Gross wrote:
> On 03/04/17 14:42, Daniel Kiper wrote:
> > On Fri, Mar 31, 2017 at 12:14:38PM +0200, Juergen Gross wrote:
> >> For kdump to work correctly it needs the physical address of
> >> vmcoreinfo_note. When running as dom0 this means the virtual address
> >> has to be translated to the related machine address.
> >>
> >> paddr_vmcoreinfo_note() is meant to do the translation via
> >> __pa_symbol() only, but being attributed "weak" it can be replaced
> >> easily in Xen case.
> >>
> >> Signed-off-by: Juergen Gross <jgr...@suse.com>
> >
> > Have you tested this patch with latest crash tool? Do dom0 and Xen
> > hypervisor analysis work without any issue (at least basic commands
> > like dmesg, bt, ps, etc.)? If yes for both you can add:
> >
> > Reviewed-by: Daniel Kiper <daniel.ki...@oracle.com>
>
> So can I add your R-b: now?

My R-b is still valid. Though, let's wait for Petr's Tested-by. He
did makedumpfile tests but I asked him to do crash tool tests too.
I think it is important.

Daniel

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

Re: [Xen-devel] [PATCH] kexec: Add spinlock for the whole hypercall

[Daniel Kiper]

On Tue, Apr 11, 2017 at 06:31:36AM -0600, Jan Beulich wrote:
> >>> On 11.04.17 at 13:24,  wrote:
> > On Tue, Apr 11, 2017 at 01:46:37AM -0600, Jan Beulich wrote:
> >> >>> On 10.04.17 at 21:44,  wrote:
> >> wouldn't it be better to handle this with a static state variable,
> >> which gets checked/set atomically, and which if already set simply
> >> leads to a continuation being arranged for?
> >
> > Do you think about something like that:
> >
> > if ( test_bit(KEXEC_FLAG_IN_PROGRESS, _flags) && 
> > hypercall_preempt_check() )
> > return hypercall_create_continuation(__HYPERVISOR_kexec_op, "h", uarg);
>
> Well, minus the hypercall_preempt_check() call of course. And I'd

OK.

> expect this to be a test_and_set_bit().

Ahhh... Right.

Eric, please try to fix it as Jan suggested.

Daniel

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

Re: [Xen-devel] [PATCH] kexec: Add spinlock for the whole hypercall

[Daniel Kiper]

On Tue, Apr 11, 2017 at 01:46:37AM -0600, Jan Beulich wrote:
> >>> On 10.04.17 at 21:44,  wrote:
>
> Please don't forget Cc-ing the maintainer(s) of the code being modified.
>
> > @@ -1187,12 +1182,22 @@ static int do_kexec_op_internal(unsigned long op,
> >  XEN_GUEST_HANDLE_PARAM(void) uarg,
> >  bool_t compat)
> >  {
> > +static DEFINE_SPINLOCK(kexec_lock);
> >  int ret = -EINVAL;
> >
> >  ret = xsm_kexec(XSM_PRIV);
> >  if ( ret )
> >  return ret;
> >
> > +/*
> > + * Because we write directly to the reserved memory
> > + * region when loading crash kernels we need a spinlock here to
> > + * prevent multiple crash kernels from attempting to load
> > + * simultaneously, and to prevent a crash kernel from loading
> > + * over the top of a in use crash kernel.
> > + */
> > +spin_lock(_lock);
> > +
> >  switch ( op )
> >  {
> >  case KEXEC_CMD_kexec_get_range:
> > @@ -1227,6 +1232,8 @@ static int do_kexec_op_internal(unsigned long op,
> >  break;
> >  }
> >
> > +spin_unlock(_lock);
> > +
> >  return ret;
> >  }
>
> How long can a kexec-op take? Are you putting systems at risk of
> the watchdog firing? Even if you don't, you put all sorts of
> operations inside a lock now which preferably should run outside
> of atomic context (memory allocation, guest memory accesses).

Facepalm! I forgot about that.

> If such a global locking approach is really necessary (i.e. if we

Potentially no but otherwise we will further complicate sufficiently
complicated code now. So, I would like to have one sync place.

> can't expect the - privileged - caller to synchronize calls properly),
> wouldn't it be better to handle this with a static state variable,
> which gets checked/set atomically, and which if already set simply
> leads to a continuation being arranged for?

Do you think about something like that:

if ( test_bit(KEXEC_FLAG_IN_PROGRESS, _flags) && 
hypercall_preempt_check() )
return hypercall_create_continuation(__HYPERVISOR_kexec_op, "h", uarg);

> Furthermore - are there problems also with e.g. loading a
> default and a crash kernel in parallel? I.e. aren't you doing more

No it should not be any issue there.

> synchronization than really necessary?

I do not think that it is very big issue here but if you wish we can fix it 
that too.

Daniel

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

Re: [Xen-devel] [PATCH v2] xen, kdump: handle pv domain in paddr_vmcoreinfo_note()

[Daniel Kiper]

On Fri, Apr 07, 2017 at 11:16:22AM +0200, Petr Tesarik wrote:
> On Wed, 5 Apr 2017 13:13:00 +0200
> Petr Tesarik <ptesa...@suse.com> wrote:
>
> > On Tue, 4 Apr 2017 12:42:53 -0700 (PDT)
> > Daniel Kiper <daniel.ki...@oracle.com> wrote:
> >
> >[...]
> > > So, if Petr did relevant tests that is nice. However, then, IMO, this
> > > patch begs Petr Tested-by.
> >
> > Actually, I tested with this patch applied on top of kernel 4.4 (SLES
> > 12 SP2). It matches what traditional Xen had always done, so I am quite
> > confident it will work with a later kernel, but to give my Tested-by,
> > let me first re-run the test on master, hopefully until today EOB.
>
> It took me much longer than anticipated (I had some trouble setting up
> the host again), but I can confirm that the patch works as expected on

No problem. I know how it works.

> top of 4.11-rc5.

Great!

> Without the patch, makedumpfile in the crash kernel complains:
>
> /proc/vmcore doesn't contain vmcoreinfo.

Though, I would like to ask you to do crash tool tests too.
Could you do that?

> With the patch applied, dumping still fails later because of an
> unrelated bug in makedumpfile, but I was able to extract the kernel
> message buffer with "makedumpfile --dump-dmesg". This already confirms
> VMCOREINFO presence and usability.

Is it Xen specific issue or more generic one?

Daniel

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

Re: [Xen-devel] [PATCH] arm64: xen: Implement EFI reset_system callback

[Daniel Kiper]

On Thu, Apr 06, 2017 at 06:22:44PM +0200, Juergen Gross wrote:
> On 06/04/17 18:06, Daniel Kiper wrote:
> > Hi Julien,
> >
> > On Thu, Apr 06, 2017 at 04:39:13PM +0100, Julien Grall wrote:
> >> Hi Daniel,
> >>
> >> On 06/04/17 16:20, Daniel Kiper wrote:
> >>> On Thu, Apr 06, 2017 at 04:38:24PM +0200, Juergen Gross wrote:
> >>>> On 06/04/17 16:27, Daniel Kiper wrote:
> >>>>> On Thu, Apr 06, 2017 at 09:32:32AM +0100, Julien Grall wrote:
> >>>>>> Hi Juergen,
> >>>>>>
> >>>>>> On 06/04/17 07:23, Juergen Gross wrote:
> >>>>>>> On 05/04/17 21:49, Boris Ostrovsky wrote:
> >>>>>>>> On 04/05/2017 02:14 PM, Julien Grall wrote:
> >>>>>>>>> The x86 code has theoritically a similar issue, altought EFI does 
> >>>>>>>>> not
> >>>>>>>>> seem to be the preferred method. I have left it unimplemented on 
> >>>>>>>>> x86 and
> >>>>>>>>> CCed Linux Xen x86 maintainers to know their view here.
> >>>>>>>>
> >>>>>>>> (+Daniel)
> >>>>>>>>
> >>>>>>>> This could be a problem for x86 as well, at least theoretically.
> >>>>>>>> xen_machine_power_off() may call pm_power_off(), which is 
> >>>>>>>> efi.reset_system.
> >>>>>>>>
> >>>>>>>> So I think we should have a similar routine there.
> >>>>>>>
> >>>>>>> +1
> >>>>>>>
> >>>>>>> I don't see any problem with such a routine added, in contrast to
> >>>>>>> potential "reboots" instead of power off without it.
> >>>>>>>
> >>>>>>> So I think this dummy xen_efi_reset_system() should be added to
> >>>>>>> drivers/xen/efi.c instead.
> >>>>>>
> >>>>>> I will resend the patch during day with xen_efi_reset_system moved
> >>>>>> to common code and implement the x86 counterpart (thought, I will
> >>>>>> not be able to test it).
> >>>>>
> >>>>> I think that this is ARM specific issue. On x86 machine_restart() calls
> >>>>> xen_restart(). Hence, everything works. So, I think that it should be
> >>>>> fixed only for ARM. Anyway, please CC me when you send a patch.
> >>>>
> >>>> What about xen_machine_power_off() (as stated in Boris' mail)?
> >>>
> >>> Guys what do you think about that:
> >>>
> >>> --- a/drivers/firmware/efi/reboot.c
> >>> +++ b/drivers/firmware/efi/reboot.c
> >>> @@ -55,7 +55,7 @@ static void efi_power_off(void)
> >>>
> >>> static int __init efi_shutdown_init(void)
> >>> {
> >>> -   if (!efi_enabled(EFI_RUNTIME_SERVICES))
> >>> +   if (!efi_enabled(EFI_RUNTIME_SERVICES) || 
> >>> efi_enabled(EFI_PARAVIRT))
> >>>return -ENODEV;
> >>>
> >>>if (efi_poweroff_required())
> >>>
> >>>
> >>> Julien, for ARM64 please take a look at 
> >>> arch/arm64/kernel/efi.c:efi_poweroff_required(void).
> >>>
> >>> I hope that tweaks for both files should solve our problem.
> >>
> >> This sounds good for power off (I haven't tried to power off DOM0
> >> yet). But this will not solve the restart problem (see
> >> machine_restart in arch/arm64/kernel/process.c) which call directly
> >> efi_reboot.
> >
> > Hmmm... It seems to me that efi.reset_system override with empty function
> > in arch/arm/xen/efi.c is the best solution. So, I see three patches here.
> > One for drivers/firmware/efi/reboot.c, one for arch/arm/xen/efi.c and one
> > for arch/arm64/kernel/efi.c. Does it make sense?
>
> I still think the empty function should be in drivers/xen/efi.c and we
> should use it in arch/x86/xen/efi.c, too.

If you wish we can go that way too. Though I thing that we should fix
drivers/firmware/efi/reboot.c:efi_shutdown_init() too. Just in case.

Daniel

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

Re: [Xen-devel] [PATCH] arm64: xen: Implement EFI reset_system callback

[Daniel Kiper]

Hi Julien,

On Thu, Apr 06, 2017 at 04:39:13PM +0100, Julien Grall wrote:
> Hi Daniel,
>
> On 06/04/17 16:20, Daniel Kiper wrote:
> >On Thu, Apr 06, 2017 at 04:38:24PM +0200, Juergen Gross wrote:
> >>On 06/04/17 16:27, Daniel Kiper wrote:
> >>>On Thu, Apr 06, 2017 at 09:32:32AM +0100, Julien Grall wrote:
> >>>>Hi Juergen,
> >>>>
> >>>>On 06/04/17 07:23, Juergen Gross wrote:
> >>>>>On 05/04/17 21:49, Boris Ostrovsky wrote:
> >>>>>>On 04/05/2017 02:14 PM, Julien Grall wrote:
> >>>>>>>The x86 code has theoritically a similar issue, altought EFI does not
> >>>>>>>seem to be the preferred method. I have left it unimplemented on x86 
> >>>>>>>and
> >>>>>>>CCed Linux Xen x86 maintainers to know their view here.
> >>>>>>
> >>>>>>(+Daniel)
> >>>>>>
> >>>>>>This could be a problem for x86 as well, at least theoretically.
> >>>>>>xen_machine_power_off() may call pm_power_off(), which is 
> >>>>>>efi.reset_system.
> >>>>>>
> >>>>>>So I think we should have a similar routine there.
> >>>>>
> >>>>>+1
> >>>>>
> >>>>>I don't see any problem with such a routine added, in contrast to
> >>>>>potential "reboots" instead of power off without it.
> >>>>>
> >>>>>So I think this dummy xen_efi_reset_system() should be added to
> >>>>>drivers/xen/efi.c instead.
> >>>>
> >>>>I will resend the patch during day with xen_efi_reset_system moved
> >>>>to common code and implement the x86 counterpart (thought, I will
> >>>>not be able to test it).
> >>>
> >>>I think that this is ARM specific issue. On x86 machine_restart() calls
> >>>xen_restart(). Hence, everything works. So, I think that it should be
> >>>fixed only for ARM. Anyway, please CC me when you send a patch.
> >>
> >>What about xen_machine_power_off() (as stated in Boris' mail)?
> >
> >Guys what do you think about that:
> >
> >--- a/drivers/firmware/efi/reboot.c
> >+++ b/drivers/firmware/efi/reboot.c
> >@@ -55,7 +55,7 @@ static void efi_power_off(void)
> >
> > static int __init efi_shutdown_init(void)
> > {
> >-   if (!efi_enabled(EFI_RUNTIME_SERVICES))
> >+   if (!efi_enabled(EFI_RUNTIME_SERVICES) || efi_enabled(EFI_PARAVIRT))
> >return -ENODEV;
> >
> >if (efi_poweroff_required())
> >
> >
> >Julien, for ARM64 please take a look at 
> >arch/arm64/kernel/efi.c:efi_poweroff_required(void).
> >
> >I hope that tweaks for both files should solve our problem.
>
> This sounds good for power off (I haven't tried to power off DOM0
> yet). But this will not solve the restart problem (see
> machine_restart in arch/arm64/kernel/process.c) which call directly
> efi_reboot.

Hmmm... It seems to me that efi.reset_system override with empty function
in arch/arm/xen/efi.c is the best solution. So, I see three patches here.
One for drivers/firmware/efi/reboot.c, one for arch/arm/xen/efi.c and one
for arch/arm64/kernel/efi.c. Does it make sense?

Daniel

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

Re: [Xen-devel] [PATCH] arm64: xen: Implement EFI reset_system callback

[Daniel Kiper]

On Thu, Apr 06, 2017 at 04:38:24PM +0200, Juergen Gross wrote:
> On 06/04/17 16:27, Daniel Kiper wrote:
> > On Thu, Apr 06, 2017 at 09:32:32AM +0100, Julien Grall wrote:
> >> Hi Juergen,
> >>
> >> On 06/04/17 07:23, Juergen Gross wrote:
> >>> On 05/04/17 21:49, Boris Ostrovsky wrote:
> >>>> On 04/05/2017 02:14 PM, Julien Grall wrote:
> >>>>> The x86 code has theoritically a similar issue, altought EFI does not
> >>>>> seem to be the preferred method. I have left it unimplemented on x86 and
> >>>>> CCed Linux Xen x86 maintainers to know their view here.
> >>>>
> >>>> (+Daniel)
> >>>>
> >>>> This could be a problem for x86 as well, at least theoretically.
> >>>> xen_machine_power_off() may call pm_power_off(), which is 
> >>>> efi.reset_system.
> >>>>
> >>>> So I think we should have a similar routine there.
> >>>
> >>> +1
> >>>
> >>> I don't see any problem with such a routine added, in contrast to
> >>> potential "reboots" instead of power off without it.
> >>>
> >>> So I think this dummy xen_efi_reset_system() should be added to
> >>> drivers/xen/efi.c instead.
> >>
> >> I will resend the patch during day with xen_efi_reset_system moved
> >> to common code and implement the x86 counterpart (thought, I will
> >> not be able to test it).
> >
> > I think that this is ARM specific issue. On x86 machine_restart() calls
> > xen_restart(). Hence, everything works. So, I think that it should be
> > fixed only for ARM. Anyway, please CC me when you send a patch.
>
> What about xen_machine_power_off() (as stated in Boris' mail)?

Guys what do you think about that:

--- a/drivers/firmware/efi/reboot.c
+++ b/drivers/firmware/efi/reboot.c
@@ -55,7 +55,7 @@ static void efi_power_off(void)

 static int __init efi_shutdown_init(void)
 {
-   if (!efi_enabled(EFI_RUNTIME_SERVICES))
+   if (!efi_enabled(EFI_RUNTIME_SERVICES) || efi_enabled(EFI_PARAVIRT))
return -ENODEV;

if (efi_poweroff_required())


Julien, for ARM64 please take a look at 
arch/arm64/kernel/efi.c:efi_poweroff_required(void).

I hope that tweaks for both files should solve our problem.

Daniel

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

Re: [Xen-devel] [PATCH] arm64: xen: Implement EFI reset_system callback

[Daniel Kiper]

On Thu, Apr 06, 2017 at 09:32:32AM +0100, Julien Grall wrote:
> Hi Juergen,
>
> On 06/04/17 07:23, Juergen Gross wrote:
> >On 05/04/17 21:49, Boris Ostrovsky wrote:
> >>On 04/05/2017 02:14 PM, Julien Grall wrote:
> >>>The x86 code has theoritically a similar issue, altought EFI does not
> >>>seem to be the preferred method. I have left it unimplemented on x86 and
> >>>CCed Linux Xen x86 maintainers to know their view here.
> >>
> >>(+Daniel)
> >>
> >>This could be a problem for x86 as well, at least theoretically.
> >>xen_machine_power_off() may call pm_power_off(), which is efi.reset_system.
> >>
> >>So I think we should have a similar routine there.
> >
> >+1
> >
> >I don't see any problem with such a routine added, in contrast to
> >potential "reboots" instead of power off without it.
> >
> >So I think this dummy xen_efi_reset_system() should be added to
> >drivers/xen/efi.c instead.
>
> I will resend the patch during day with xen_efi_reset_system moved
> to common code and implement the x86 counterpart (thought, I will
> not be able to test it).

I think that this is ARM specific issue. On x86 machine_restart() calls
xen_restart(). Hence, everything works. So, I think that it should be
fixed only for ARM. Anyway, please CC me when you send a patch.

Daniel

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

Re: [Xen-devel] [PATCH v2] xen, kdump: handle pv domain in paddr_vmcoreinfo_note()

[Daniel Kiper]

> On 03/04/17 14:42, Daniel Kiper wrote:
> > On Fri, Mar 31, 2017 at 12:14:38PM +0200, Juergen Gross wrote:
> >> For kdump to work correctly it needs the physical address of
> >> vmcoreinfo_note. When running as dom0 this means the virtual address
> >> has to be translated to the related machine address.
> >>
> >> paddr_vmcoreinfo_note() is meant to do the translation via
> >> __pa_symbol() only, but being attributed "weak" it can be replaced
> >> easily in Xen case.
> >>
> >> Signed-off-by: Juergen Gross <jgr...@suse.com>
> >
> > Have you tested this patch with latest crash tool? Do dom0 and Xen
> > hypervisor analysis work without any issue (at least basic commands
> > like dmesg, bt, ps, etc.)? If yes for both you can add:
> >
> > Reviewed-by: Daniel Kiper <daniel.ki...@oracle.com>
>
> This patch isn't for dump analysis, but for dump creation. Petr has

I know that. However, it may have impact on crash analysis. So,
I would expect that you or anybody else in your behalf will do
at least minimal crash tool tests.

> verified that the dump is in the expected format. Please ask Petr
> for further details, e.g. user side modifications being necessary.

So, if Petr did relevant tests that is nice. However, then, IMO, this
patch begs Petr Tested-by.

Daniel

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

Re: [Xen-devel] [PATCH] kexec: clear kexec_image slot when unloading kexec image

[Daniel Kiper]

On Mon, Apr 03, 2017 at 10:44:54AM -0700, Bhavesh Davda wrote:
> While theoretically this bug can be tickled simply by a sequence of 'kexec -p'
> to load a kexec crash image followed by two back-to-back 'kexec -p -u' to
> unload the kexec crash image, I found the following perl script to be useful 
> to
> reliably reproduce Xen panics as well as verify that the fix works. YMMV.
>
> -snip-
> #!/usr/bin/perl -w
>
> use strict;
> use warnings;
> use threads;
>
> sub threaded_task {
> threads->create(sub {
> my $thr_id = threads->self->tid;
> print "Starting load thread $thr_id\n";
> system("/sbin/kexec  -p --command-line=\"placeholder 
> root=/dev/mapper/root ro rhbg console=tty0 console=hvc0 earlyprintk=xen 
> nomodeset printk.time=1 irqpoll maxcpus=1 nr_cpus=1 reset_devices 
> cgroup_disable=memory mce=off selinux=0 console=ttyS1,115200n8\" 
> --initrd=/boot/initrd.x86_64kdump.img /boot/vmlinuz.x86_64");
> print "Ending load thread $thr_id\n";
> threads->detach(); #End thread.
> });
> threads->create(sub {
> my $thr_id = threads->self->tid;
> print "Starting unload thread $thr_id\n";
> system("/sbin/kexec  -p -u");
> print "Ending unload thread $thr_id\n";
> threads->detach(); #End thread.
> });
> }
>
> for my $i (0..99)
> {
> threaded_task();
> }
> -snip-
>
> ---
> When kexec_do_unload calls kexec_swap_images to get the old kexec_image to
> free, it passes NULL for the new kexec_image pointer. The new slot wasn't 
> being
> cleared in such a case, leading to a stale pointer being left behind in the
> kexec_image array and Xen panics in subsequent load/unload operations.
>
> Signed-off-by: Bhavesh Davda <bhavesh.da...@oracle.com>
> Reviewed-by: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
> Reviewed-by: Daniel Kiper <daniel.ki...@oracle.com>
> ---
>  xen/common/kexec.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/xen/common/kexec.c b/xen/common/kexec.c
> index 940fc7ec94..072cc8e0db 100644
> --- a/xen/common/kexec.c
> +++ b/xen/common/kexec.c
> @@ -837,11 +837,9 @@ static int kexec_swap_images(int type, struct 
> kexec_image *new,
>  old_slot = base + pos;
>  new_slot = base + !pos;
>
> +kexec_image[new_slot] = new;
>  if ( new )
> -{
> -kexec_image[new_slot] = new;
>  set_bit(new_slot, _flags);
> -}
>  change_bit(bit, _flags);
>
>  clear_bit(old_slot, _flags);

Bhavesh, thanks for posting this.

Jan, Andrew, IMO, this is Xen stable material too.

Daniel

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

Re: [Xen-devel] [PATCH v2] xen, kdump: handle pv domain in paddr_vmcoreinfo_note()

[Daniel Kiper]

On Fri, Mar 31, 2017 at 12:14:38PM +0200, Juergen Gross wrote:
> For kdump to work correctly it needs the physical address of
> vmcoreinfo_note. When running as dom0 this means the virtual address
> has to be translated to the related machine address.
>
> paddr_vmcoreinfo_note() is meant to do the translation via
> __pa_symbol() only, but being attributed "weak" it can be replaced
> easily in Xen case.
>
> Signed-off-by: Juergen Gross <jgr...@suse.com>

Have you tested this patch with latest crash tool? Do dom0 and Xen
hypervisor analysis work without any issue (at least basic commands
like dmesg, bt, ps, etc.)? If yes for both you can add:

Reviewed-by: Daniel Kiper <daniel.ki...@oracle.com>

Daniel

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

Re: [Xen-devel] [PATCH 0/3] xen: support of large memory maps

[Daniel Kiper]

On Tue, Mar 21, 2017 at 02:10:20PM +0100, Juergen Gross wrote:
> This patch series is the first part for adding support of large EFI
> memory maps (> the current limit of 128 entries) while reducing
> trampoline size.
>
> I'm not posting the final patch for making the trampoline size
> reduction effective in order not to add major rebase work to Daniel's
> multiboot2 series.

Thanks a lot Juergen!

Daniel

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

Re: [Xen-devel] Supporting systems with large E820 maps

[Daniel Kiper]

On Tue, Mar 21, 2017 at 01:01:44PM +0100, Juergen Gross wrote:
> On 21/03/17 11:05, Jan Beulich wrote:
>  On 21.03.17 at 06:14,  wrote:
> >> On 20/03/17 20:03, Alex Thorlton wrote:
> >>> Hey everyone,
> >>>
> >>> Recently, I've been working with Boris Ostrovsky to get Xen running on
> >>> some of our larger systems, and we've run into a few problems with the
> >>> amount of space that Xen sets aside for the E820 map.
> >>>
> >>> The first problem that I hit was that E820MAX is far too small, at 128
> >>> entries, for the system that we're testing with.  The EFI memory map
> >>> handed up from the boot loader tops out at 783 entries, which far
> >>> exceeds the amount of space allocated for the memory map in
> >>> arch/x86/boot/mem.S.  I was able to get past this problem by bumping
> >>> E820MAX up to 1024 in arch/x86/boot/mem.S and include/asm-x86/e820.h.
> >>>
> >>> The second problem that I encountered was that Xen uses a signed char to
> >>> store the number of entries in the memory map in a few places, which is
> >>> too small to hold the number of entries after bumping E820MAX up to
> >>> 1024.  I made the following changes to get past this:
> >>
> >> The problem with setting E820MAX to a higher value in mem.S without
> >> further measures is that you are growing the trampoline size. This is
> >> problematic for memory allocation in the multiboot path.
> >>
> >> I have some patches sitting here waiting for Daniel's multiboot series
> >> to go in. My patches are not using the mem.S e820 array for the EFI
> >> memory map, so the BIOS memory map buffer can remain smaller while the
> >> EFI buffer can be made rather large. This avoids growing the trampoline
> >> (in fact I've managed to reduce it to a single page).
> >>
> >> I didn't post my series up to now in order to not block Daniel's series
> >> again. So what do people think: should I wait some more time with my
> >> patches, or should I send them rather soon?
> >
> > I'd say go ahead - whether the rest of Daniel's series will go in for
> > 4.9 is undetermined at this point in time. At the very least we first
> > need to get details on the boot regression Andrew says they're
> > observing with what has gone in already.
>
> Okay. I think I'll just post the first three patches which basically
> will support more EFI memory map entries without affecting the
> assembler parts too much.

This is not a problem for me in general. However, if you can try to not
touch much of the same code as I do then it will be nice. And of course
if you CC me I will be more than happy.

Daniel

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

Re: [Xen-devel] [PATCH v16 4/9] x86: add multiboot2 protocol support for EFI platforms

[Daniel Kiper]

On Thu, Mar 16, 2017 at 07:43:57AM -0600, Jan Beulich wrote:
> >>> On 16.03.17 at 14:35,  wrote:
> > On Thu, Mar 16, 2017 at 07:12:21AM -0600, Jan Beulich wrote:
> >> >>> On 16.03.17 at 13:12,  wrote:
> >> > Everything works. I am not able to reproduce any issues reported by Doug.
> >> > Andrew, Jan, Doug, are there still any objections to commit the rest of
> > v16?
> >>
> >> Well, Andrew has been telling me privately that even the part that
> >> was already committed regresses XenServer in some way.
> >
> > Hmmm... Andrew, could you provide more details?
> >
> >> Independent of that, Doug, can you confirm things work for you
> >> with the remaining patches applied to current staging?
> >>
> >> And of course patch 7 still needs looking at, which I didn't make a
> >> priority because so far it looked like there'll be fixes needed anyway.
> >
> > OK, so, there is a question here: When is Xen 4.9 final call for this
> > series?
> > Is it last posting date or hard code freeze?
>
> The latter, afaik.

Guys, tempus fugit. Andrew, could you post details about discovered issues?
Doug, is it OK for you to get the rest of my patches into master? Jan, could
you review the rest of v16 patches? I really, really, really, wish to see
this stuff in 4.9.

Daniel

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

Re: [Xen-devel] [PATCH v16 4/9] x86: add multiboot2 protocol support for EFI platforms

[Daniel Kiper]

On Thu, Mar 16, 2017 at 07:12:21AM -0600, Jan Beulich wrote:
> >>> On 16.03.17 at 13:12,  wrote:
> > Everything works. I am not able to reproduce any issues reported by Doug.
> > Andrew, Jan, Doug, are there still any objections to commit the rest of v16?
>
> Well, Andrew has been telling me privately that even the part that
> was already committed regresses XenServer in some way.

Hmmm... Andrew, could you provide more details?

> Independent of that, Doug, can you confirm things work for you
> with the remaining patches applied to current staging?
>
> And of course patch 7 still needs looking at, which I didn't make a
> priority because so far it looked like there'll be fixes needed anyway.

OK, so, there is a question here: When is Xen 4.9 final call for this series?
Is it last posting date or hard code freeze?

Daniel

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

Re: [Xen-devel] [PATCH v16 4/9] x86: add multiboot2 protocol support for EFI platforms

[Daniel Kiper]

On Wed, Mar 15, 2017 at 09:27:27AM -0500, Doug Goldstein wrote:
> On 3/15/17 6:35 AM, Daniel Kiper wrote:
> > On Thu, Mar 09, 2017 at 02:02:49PM -0600, Doug Goldstein wrote:
> >
> > [...]
> >
> >> Still missing 'xl info'.
> >
> > Got Intel NUC5i3MYHE (internally it is NUC5i3MYBE board) into my hands.
> > I have put 8 GiB RAM and 500 GB SATA 3 into it. Updated BIOS/EFI to 0041
> > version (it is the latest one). Installed latest Debian testing (Debian
> > GNU/Linux 9 (stretch)), built GRUB2 and Xen, with and without relocation
> > patches, on it. Everything works (I left machine working last night).
> > Guest boots without any issue. Please take look at attached logs.
> >
> > Doug, could you tell me how exactly did you test your machine? I need OS
> > type, version, C version (GCC, clang, anything else), bintuils version,
> > etc. "xl dmesg", "xl info" and "dmesg" full outputs are welcome too.
> >
> > Daniel
> >
>
> I thought I already responded to Konrad saying that latest staging +
> relocation patches also comes up.
>
> My guess is that it is related to the IOMMU "fix" that Andrew and Jan
> did by #if 0'ing out some of ebmalloc. But I'm not sure. I haven't had
> time to look at any of this stuff lately. I went to ELC and then
> vacation and then managed to hurt myself on vacation so I've been away
> from my computer a bit.
>
> All my branches are available in https://github.com/cardoe/xen and I've
> been on Ubuntu 16.04.

Everything works. I am not able to reproduce any issues reported by Doug.
Andrew, Jan, Doug, are there still any objections to commit the rest of v16?

Daniel

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

Re: [Xen-devel] [PATCH v16 4/9] x86: add multiboot2 protocol support for EFI platforms

[Daniel Kiper]

On Wed, Mar 15, 2017 at 09:42:53AM -0500, Doug Goldstein wrote:
> On 3/15/17 9:38 AM, Daniel Kiper wrote:
> > On Wed, Mar 15, 2017 at 09:27:27AM -0500, Doug Goldstein wrote:
> >> On 3/15/17 6:35 AM, Daniel Kiper wrote:
> >>> On Thu, Mar 09, 2017 at 02:02:49PM -0600, Doug Goldstein wrote:
> >>>
> >>> [...]
> >>>
> >>>> Still missing 'xl info'.
> >>>
> >>> Got Intel NUC5i3MYHE (internally it is NUC5i3MYBE board) into my hands.
> >>> I have put 8 GiB RAM and 500 GB SATA 3 into it. Updated BIOS/EFI to 0041
> >>> version (it is the latest one). Installed latest Debian testing (Debian
> >>> GNU/Linux 9 (stretch)), built GRUB2 and Xen, with and without relocation
> >>> patches, on it. Everything works (I left machine working last night).
> >>> Guest boots without any issue. Please take look at attached logs.
> >>>
> >>> Doug, could you tell me how exactly did you test your machine? I need OS
> >>> type, version, C version (GCC, clang, anything else), bintuils version,
> >>> etc. "xl dmesg", "xl info" and "dmesg" full outputs are welcome too.
> >>>
> >>> Daniel
> >>
> >> I thought I already responded to Konrad saying that latest staging +
> >> relocation patches also comes up.
> >
> > I do not remember it. Maybe I have missed that.
> >
> >> My guess is that it is related to the IOMMU "fix" that Andrew and Jan
> >> did by #if 0'ing out some of ebmalloc. But I'm not sure. I haven't had
> >
> > I reenabled free_ebmalloc_unused_mem() during QEMU tests last week.
> > It has not changed anything. I will do the same on my NUC.
> >
> >> time to look at any of this stuff lately. I went to ELC and then
> >> vacation and then managed to hurt myself on vacation so I've been away
> >> from my computer a bit.
> >>
> >> All my branches are available in https://github.com/cardoe/xen and I've
> >> been on Ubuntu 16.04.
> >
> > I will try this too. Thanks for update.
> >
> > Daniel
>
> Where's the branch you are using on that NUC? Because you can't be using
> plain staging on there because the firmware version you reported dead
> locks when the EFI GetTime() is called. Its a known issue by Intel. So
> you must be patching that out of your Xen?

I rebased relocation patches (v16) on top of 
9dc1e0cd81ee469d638d1962a92d9b4bd2972bfa
(x86/pagewalk: Consistently use guest_walk_*() helpers for translation) commit.
No more no less. Everything works. Am I missing something?

Daniel

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

Re: [Xen-devel] [PATCH v16 4/9] x86: add multiboot2 protocol support for EFI platforms

[Daniel Kiper]

On Wed, Mar 15, 2017 at 09:27:27AM -0500, Doug Goldstein wrote:
> On 3/15/17 6:35 AM, Daniel Kiper wrote:
> > On Thu, Mar 09, 2017 at 02:02:49PM -0600, Doug Goldstein wrote:
> >
> > [...]
> >
> >> Still missing 'xl info'.
> >
> > Got Intel NUC5i3MYHE (internally it is NUC5i3MYBE board) into my hands.
> > I have put 8 GiB RAM and 500 GB SATA 3 into it. Updated BIOS/EFI to 0041
> > version (it is the latest one). Installed latest Debian testing (Debian
> > GNU/Linux 9 (stretch)), built GRUB2 and Xen, with and without relocation
> > patches, on it. Everything works (I left machine working last night).
> > Guest boots without any issue. Please take look at attached logs.
> >
> > Doug, could you tell me how exactly did you test your machine? I need OS
> > type, version, C version (GCC, clang, anything else), bintuils version,
> > etc. "xl dmesg", "xl info" and "dmesg" full outputs are welcome too.
> >
> > Daniel
> >
>
> I thought I already responded to Konrad saying that latest staging +
> relocation patches also comes up.

I do not remember it. Maybe I have missed that.

> My guess is that it is related to the IOMMU "fix" that Andrew and Jan
> did by #if 0'ing out some of ebmalloc. But I'm not sure. I haven't had

I reenabled free_ebmalloc_unused_mem() during QEMU tests last week.
It has not changed anything. I will do the same on my NUC.

> time to look at any of this stuff lately. I went to ELC and then
> vacation and then managed to hurt myself on vacation so I've been away
> from my computer a bit.
>
> All my branches are available in https://github.com/cardoe/xen and I've
> been on Ubuntu 16.04.

I will try this too. Thanks for update.

Daniel

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

Re: [Xen-devel] [PATCH v16 4/9] x86: add multiboot2 protocol support for EFI platforms

[Daniel Kiper]

On Thu, Mar 09, 2017 at 02:02:49PM -0600, Doug Goldstein wrote:

[...]

> Still missing 'xl info'.

Got Intel NUC5i3MYHE (internally it is NUC5i3MYBE board) into my hands.
I have put 8 GiB RAM and 500 GB SATA 3 into it. Updated BIOS/EFI to 0041
version (it is the latest one). Installed latest Debian testing (Debian
GNU/Linux 9 (stretch)), built GRUB2 and Xen, with and without relocation
patches, on it. Everything works (I left machine working last night).
Guest boots without any issue. Please take look at attached logs.

Doug, could you tell me how exactly did you test your machine? I need OS
type, version, C version (GCC, clang, anything else), bintuils version,
etc. "xl dmesg", "xl info" and "dmesg" full outputs are welcome too.

Daniel
[0.00] Linux version 4.9.0-2-amd64 (debian-ker...@lists.debian.org) 
(gcc version 6.3.0 20170221 (Debian 6.3.0-8) ) #1 SMP Debian 4.9.13-1 
(2017-02-27)
[0.00] Command line: placeholder root=/dev/mapper/vg00-root ro quiet
[0.00] x86/fpu: Supporting XSAVE feature 0x001: 'x87 floating point 
registers'
[0.00] x86/fpu: Supporting XSAVE feature 0x002: 'SSE registers'
[0.00] x86/fpu: Supporting XSAVE feature 0x004: 'AVX registers'
[0.00] x86/fpu: xstate_offset[2]:  576, xstate_sizes[2]:  256
[0.00] x86/fpu: Enabled xstate features 0x7, context size is 832 bytes, 
using 'standard' format.
[0.00] x86/fpu: Using 'eager' FPU context switches.
[0.00] Released 0 page(s)
[0.00] e820: BIOS-provided physical RAM map:
[0.00] Xen: [mem 0x-0x00057fff] usable
[0.00] Xen: [mem 0x00058000-0x00058fff] reserved
[0.00] Xen: [mem 0x00059000-0x0009dfff] usable
[0.00] Xen: [mem 0x0009e000-0x000f] reserved
[0.00] Xen: [mem 0x0010-0x9d8c0fff] usable
[0.00] Xen: [mem 0x9d8c1000-0x9dd80fff] reserved
[0.00] Xen: [mem 0x9dd81000-0xa2283fff] usable
[0.00] Xen: [mem 0xa2284000-0xa233efff] reserved
[0.00] Xen: [mem 0xa233f000-0xa2363fff] ACPI data
[0.00] Xen: [mem 0xa2364000-0xa2c93fff] ACPI NVS
[0.00] Xen: [mem 0xa2c94000-0xa2ffefff] reserved
[0.00] Xen: [mem 0xa2fff000-0xa2ff] usable
[0.00] Xen: [mem 0xa380-0xa7ff] reserved
[0.00] Xen: [mem 0xf800-0xfbff] reserved
[0.00] Xen: [mem 0xfec0-0xfec00fff] reserved
[0.00] Xen: [mem 0xfed0-0xfed03fff] reserved
[0.00] Xen: [mem 0xfed1c000-0xfed1] reserved
[0.00] Xen: [mem 0xfed9-0xfed91fff] reserved
[0.00] Xen: [mem 0xfee0-0xfeef] reserved
[0.00] Xen: [mem 0xff00-0x] reserved
[0.00] Xen: [mem 0x0001-0x00015e29dfff] usable
[0.00] NX (Execute Disable) protection: active
[0.00] efi: EFI v2.40 by American Megatrends
[0.00] efi:  ACPI=0xa2346000  ACPI 2.0=0xa2346000  ESRT=0xa2ee7b98  
SMBIOS=0xa2ee7d18 
[0.00] SMBIOS 2.8 present.
[0.00] DMI:  /NUC5i3MYBE, BIOS 
MYBDWi30.86A.0041.2017.0116.2158 01/16/2017
[0.00] Hypervisor detected: Xen
[0.00] e820: update [mem 0x-0x0fff] usable ==> reserved
[0.00] e820: remove [mem 0x000a-0x000f] usable
[0.00] e820: last_pfn = 0x15e29e max_arch_pfn = 0x4
[0.00] MTRR: Disabled
[0.00] x86/PAT: MTRRs disabled, skipping PAT initialization too.
[0.00] x86/PAT: Configuration [0-7]: WB  WT  UC- UC  WC  WP  UC  UC  
[0.00] e820: last_pfn = 0xa3000 max_arch_pfn = 0x4
[0.00] Base memory trampoline at [88098000] 98000 size 24576
[0.00] BRK [0x01f2b000, 0x01f2bfff] PGTABLE
[0.00] BRK [0x01f2c000, 0x01f2cfff] PGTABLE
[0.00] BRK [0x01f2d000, 0x01f2dfff] PGTABLE
[0.00] BRK [0x01f2e000, 0x01f2efff] PGTABLE
[0.00] BRK [0x01f2f000, 0x01f2] PGTABLE
[0.00] BRK [0x01f3, 0x01f30fff] PGTABLE
[0.00] BRK [0x01f31000, 0x01f31fff] PGTABLE
[0.00] BRK [0x01f32000, 0x01f32fff] PGTABLE
[0.00] BRK [0x01f33000, 0x01f33fff] PGTABLE
[0.00] BRK [0x01f34000, 0x01f34fff] PGTABLE
[0.00] BRK [0x01f35000, 0x01f35fff] PGTABLE
[0.00] BRK [0x01f36000, 0x01f36fff] PGTABLE
[0.00] RAMDISK: [mem 0x0200-0x03263fff]
[0.00] ACPI: Early table checksum verification disabled
[0.00] ACPI: RSDP 0xA2346000 24 (v02 INTEL )
[0.00] ACPI: XSDT 0xA2346098 B4 (v01 INTEL  NUC5i3MY 
01072009 AMI  00010013)
[0.00] ACPI: FACP 0xA235BA30 00010C (v05 INTEL  NUC5i3MY 
01072009 AMI  00010013)
[0.00] ACPI: 

Re: [Xen-devel] [RFC PATCH] mm, hotplug: get rid of auto_online_blocks

[Daniel Kiper]

Hey,

On Mon, Mar 06, 2017 at 03:54:17PM +0100, Michal Hocko wrote:

[...]

> So let's discuss the current memory hotplug shortcomings and get rid of
> the crud which developed on top. I will start by splitting up the patch
> into 3 parts. Do the auto online thing from the HyperV and xen balloning
> drivers and dropping the config option and finally drop the sysfs knob.
> The last patch might be NAKed and I can live with that as long as the
> reasoning is proper and there is a general consensus on that.

If it is not too late please CC me on the patches and relevant threads.

Daniel

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

Re: [Xen-devel] [PATCH v16 4/9] x86: add multiboot2 protocol support for EFI platforms

[Daniel Kiper]

On Tue, Mar 07, 2017 at 12:39:04AM +0100, Daniel Kiper wrote:
> On Wed, Feb 22, 2017 at 09:04:17AM -0800, Doug Goldstein wrote:
>
> [...]
>
> > I'm currently at ELC and then on vacation so I don't have access to any
> > of the machines currently myself. However the machine I most use to test
> > is a NUC5i5MYHE and a NUC5i3MYHE if you want to ask around if someone
> > has one internally. But that's why I gave QEMU as an example.
> >
> > I was using qemu master from a few weeks ago. I'll have to find the
> > revision for you. But the command line I use is:
> >
> > -enable-kvm -M pc-q35-2.8 -device intel-iommu -cpu host -m 2048 -smp 2
> > -drive if=pflash,format=raw,file=/tmp/tmp.EiR6ixmYzV -global
> > isa-debugcon.iobase=0x402 -debugcon file:/tmp/tmp.nuvEXUWfnA -monitor
> > stdio -chardev socket,host=127.0.0.1,port=25914,id=S0,server,nowait
> > -device isa-serial,chardev=S0 -device piix3-usb-uhci -device usb-tablet
> > -netdev id=net0,type=tap -device
> > virtio-net-pci,netdev=net0,mac=52:54:00:12:34:56 -boot order=n -device
> > qxl-vga -gdb tcp::14952
>
> Sadly, my colleagues and I are not able to reproduce the problem on any of
> machines available for us (available on the market and some development
> stuff in our labs). I did tests with QEMU (I am not able to run it with
> "-device intel-iommu" on my machine; I have to investigate this). Everything
> works. Joao did some tests on Intel NUC D34010WYK second generation.
> Everything works. So, Konrad ordered Intel NUC NUC5i3MYHE for me. I am
> waiting for delivery. Doug, could you tell me what distro, Xen, etc. you
> have installed on that NUC? I would like to test same config as yours on
> this machine.

Latest QEMU from git with intel-iommu device enabled works without any issue.
Though it looks that I found a bug in Xen IOMMU code. If I run Xen from master
in QEMU then QEMU complains and crashes:

  qemu-system-x86_64: /srv/dev/qemu/qemu_20170308/hw/i386/intel_iommu.c:1786: 
vtd_mem_write: Assertion `size == 4' failed.
  Aborted (core dumped)

So, I took a look at dma_msi_set_affinity() and found this:

  dmar_writeq(iommu->reg, DMAR_FEADDR_REG, msg.address);

It looks bogus because DMAR_FEADDR_REG is defined in spec as 32-bit wide.
So, applied this patch:

-dmar_writeq(iommu->reg, DMAR_FEADDR_REG, msg.address);
+dmar_writel(iommu->reg, DMAR_FEADDR_REG, msg.address_lo);
+if (x2apic_enabled)
+dmar_writel(iommu->reg, DMAR_FEUADDR_REG, msg.address_hi);

Ant it looks that right now everything works.

If patch make sense I can post it with proper commit message.

Daniel

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

Re: [Xen-devel] [PATCH v16 4/9] x86: add multiboot2 protocol support for EFI platforms

[Daniel Kiper]

On Tue, Mar 07, 2017 at 10:44:15PM -0500, Konrad Rzeszutek Wilk wrote:
> On Tue, Mar 07, 2017 at 12:39:04AM +0100, Daniel Kiper wrote:
> > On Wed, Feb 22, 2017 at 09:04:17AM -0800, Doug Goldstein wrote:
> >
> > [...]
> >
> > > I'm currently at ELC and then on vacation so I don't have access to any
> > > of the machines currently myself. However the machine I most use to test
> > > is a NUC5i5MYHE and a NUC5i3MYHE if you want to ask around if someone
> > > has one internally. But that's why I gave QEMU as an example.
> > >
> > > I was using qemu master from a few weeks ago. I'll have to find the
> > > revision for you. But the command line I use is:
> > >
> > > -enable-kvm -M pc-q35-2.8 -device intel-iommu -cpu host -m 2048 -smp 2
> > > -drive if=pflash,format=raw,file=/tmp/tmp.EiR6ixmYzV -global
> > > isa-debugcon.iobase=0x402 -debugcon file:/tmp/tmp.nuvEXUWfnA -monitor
> > > stdio -chardev socket,host=127.0.0.1,port=25914,id=S0,server,nowait
> > > -device isa-serial,chardev=S0 -device piix3-usb-uhci -device usb-tablet
> > > -netdev id=net0,type=tap -device
> > > virtio-net-pci,netdev=net0,mac=52:54:00:12:34:56 -boot order=n -device
> > > qxl-vga -gdb tcp::14952
> >
> > Sadly, my colleagues and I are not able to reproduce the problem on any of
> > machines available for us (available on the market and some development
> > stuff in our labs). I did tests with QEMU (I am not able to run it with
> > "-device intel-iommu" on my machine; I have to investigate this). Everything
> > works. Joao did some tests on Intel NUC D34010WYK second generation.
> > Everything works. So, Konrad ordered Intel NUC NUC5i3MYHE for me. I am
> > waiting for delivery. Doug, could you tell me what distro, Xen, etc. you
> > have installed on that NUC? I would like to test same config as yours on
> > this machine.
>
> I had a chat with Doug on IRC and:
>  - I had tested earlier on AMD, while he has only Intel boxes,
>  - He was wondering if this was an IOMMU issue.

I had and still have a feeling that it can be related to IOMMU. I will
try to reproduce this on QEMU but first of all I have to check how to
enable this option in my environment (something is broken). Additionally,
there is a chance that the issue spotted by osstest (fixed right now)
played a role here too. So, it will be nice if Doug can do tests with
latest master. Anyway, I will do tests too. Though I am still waiting
for my NUC.

> So to double-check that, I installed Ubuntu 16.10 on my X11SAE
> SuperMicro, which has an Haswell E3-1245 v5 and with IOMMU enabled.
>
> I tested the 'origin/staging' xen.gz build with the upstream grub2
> (I just used the 'master' branch) first and also just booting xen.efi.
>
> Both worked fine.
>
> Then I used v16 of Daniel's patches (this thread). They are also
> now ongit://xenbits.xen.org/people/konradwilk/xen.git mb2.v16
> also the same way - as xen.efi and then using grub.efi and booting it
> (see below)
>
> All worked fine.

Great! Thanks a lot for doing the tests.

> Now in the process I discovered that my patch for grub-mkconfig to
> detect multiboot2 payloads and use those instead of multiboot never
> made it upstream, so I had to modify my grub.cfg by hand (see below).

It will be nice if you post it after GRUB2 2.02 release.

Daniel

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

Re: [Xen-devel] [PATCH v16 4/9] x86: add multiboot2 protocol support for EFI platforms

[Daniel Kiper]

On Wed, Feb 22, 2017 at 09:04:17AM -0800, Doug Goldstein wrote:

[...]

> I'm currently at ELC and then on vacation so I don't have access to any
> of the machines currently myself. However the machine I most use to test
> is a NUC5i5MYHE and a NUC5i3MYHE if you want to ask around if someone
> has one internally. But that's why I gave QEMU as an example.
>
> I was using qemu master from a few weeks ago. I'll have to find the
> revision for you. But the command line I use is:
>
> -enable-kvm -M pc-q35-2.8 -device intel-iommu -cpu host -m 2048 -smp 2
> -drive if=pflash,format=raw,file=/tmp/tmp.EiR6ixmYzV -global
> isa-debugcon.iobase=0x402 -debugcon file:/tmp/tmp.nuvEXUWfnA -monitor
> stdio -chardev socket,host=127.0.0.1,port=25914,id=S0,server,nowait
> -device isa-serial,chardev=S0 -device piix3-usb-uhci -device usb-tablet
> -netdev id=net0,type=tap -device
> virtio-net-pci,netdev=net0,mac=52:54:00:12:34:56 -boot order=n -device
> qxl-vga -gdb tcp::14952

Sadly, my colleagues and I are not able to reproduce the problem on any of
machines available for us (available on the market and some development
stuff in our labs). I did tests with QEMU (I am not able to run it with
"-device intel-iommu" on my machine; I have to investigate this). Everything
works. Joao did some tests on Intel NUC D34010WYK second generation.
Everything works. So, Konrad ordered Intel NUC NUC5i3MYHE for me. I am
waiting for delivery. Doug, could you tell me what distro, Xen, etc. you
have installed on that NUC? I would like to test same config as yours on
this machine.

Daniel

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

Re: [Xen-devel] [PATCH v16 5/9] x86: change default load address from 1 MiB to 2 MiB

[Daniel Kiper]

On Wed, Mar 01, 2017 at 04:21:35AM -0700, Jan Beulich wrote:
> >>> On 01.03.17 at 11:51,  wrote:
> > On Wed, Mar 01, 2017 at 03:34:47AM -0700, Jan Beulich wrote:
> >> >>> On 01.03.17 at 11:13,  wrote:
> >> > On Wed, Mar 01, 2017 at 02:05:39AM -0700, Jan Beulich wrote:
> >> >> >>> On 21.02.17 at 20:19,  wrote:
> >> >> > Subsequent patches introducing relocatable early boot code play with
> >> >> > page tables using 2 MiB huge pages. If load address is not aligned at
> >> >> > 2 MiB then code touching such page tables must have special cases for
> >> >> > start and end of Xen image memory region. So, let's make life easier
> >> >> > and move default load address from 1 MiB to 2 MiB. This way page table
> >> >> > code will be nice and easy. Hence, there is a chance that it will be
> >> >> > less error prone too... :-)))
> >> >> >
> >> >> > Additionally, drop first 2 MiB mapping from Xen image mapping.
> >> >> > It is no longer needed.
> >> >>
> >> >> What about the memory allocated for it? Aiui at least in the xen.efi
> >> >> case there would be a 2Mb chunk left unused, but also never freed.
> >> >
> >> > Memory is not allocated for it.
> >>
> >> Are you sure? Where would the PE32+ header live in that case?
> >
> > Is the PE32+ header loaded into memory? I think that only code and data 
> > stuff
> > is put there. Header is useful only for loader. Am I missing something?
>
> I think EFI's loader simply follows Windows' one(s). And yes, the

Sadly, I do not know how Windows' one(s) work(s).

> headers can be useful - to the loader itself (albeit then the loader
> assumes that no-one fiddles with them).

I have looked at OVMF. It looks that OVMF loads PE32+ header aside, parses it, 
puts
Xen code and data into allocated memory (EfiLoaderCode type), does relocations 
and
jumps into entry point. So, more or less as I expected. It looks that it does 
not
load whole file at once. Hence, taking into account that memory allocated for 
Xen
image is marked as EfiLoaderCode it means that from Xen point of view this 
region
is free. Of course later everything between _start and _end is reserved. So, it
seems to me that everything between __image_base__ and _start is free and
available for Xen memory allocator. Am I missing something here?

Daniel

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

Re: [Xen-devel] [xen-unstable bisection] complete test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm

[Daniel Kiper]

On Thu, Mar 02, 2017 at 10:42:57AM +, Andrew Cooper wrote:
> On 02/03/17 10:41, Daniel Kiper wrote:
> > On Wed, Mar 01, 2017 at 11:53:52PM +, osstest service owner wrote:
> >> branch xen-unstable
> >> xenbranch xen-unstable
> >> job test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm
> >> testid xen-boot
> >>
> >> Tree: linux git://xenbits.xen.org/linux-pvops.git
> >> Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git
> >> Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git
> >> Tree: qemuu git://xenbits.xen.org/qemu-xen.git
> >> Tree: xen git://xenbits.xen.org/xen.git
> >>
> >> *** Found and reproduced problem changeset ***
> >>
> >>   Bug is in tree:  xen git://xenbits.xen.org/xen.git
> >>   Bug introduced:  c5b9805bc1f79319ae342c65fcc201a15a47
> >>   Bug not present: b199c44afa3a0d18d0e968e78a590eb9e69e20ad
> >>   Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/106324/
> >>
> >>
> >>   commit c5b9805bc1f79319ae342c65fcc201a15a47
> >>   Author: Daniel Kiper <daniel.ki...@oracle.com>
> >>   Date:   Wed Feb 22 14:38:06 2017 +0100
> >>
> >>   efi: create new early memory allocator
> > Does anybody know why this still fails? Andrew applied a fix (workaround) 
> > for this yesterday.
>
> Bisection runs from before will still be running.  Also, the fix hasn't

This is what I expected.

> passed staging yet.

Yep, I saw that.

Thanks for update.

Daniel

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

Re: [Xen-devel] [xen-unstable bisection] complete test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm

[Daniel Kiper]

On Wed, Mar 01, 2017 at 11:53:52PM +, osstest service owner wrote:
> branch xen-unstable
> xenbranch xen-unstable
> job test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm
> testid xen-boot
>
> Tree: linux git://xenbits.xen.org/linux-pvops.git
> Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git
> Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git
> Tree: qemuu git://xenbits.xen.org/qemu-xen.git
> Tree: xen git://xenbits.xen.org/xen.git
>
> *** Found and reproduced problem changeset ***
>
>   Bug is in tree:  xen git://xenbits.xen.org/xen.git
>   Bug introduced:  c5b9805bc1f79319ae342c65fcc201a15a47
>   Bug not present: b199c44afa3a0d18d0e968e78a590eb9e69e20ad
>   Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/106324/
>
>
>   commit c5b9805bc1f79319ae342c65fcc201a15a47
>   Author: Daniel Kiper <daniel.ki...@oracle.com>
>   Date:   Wed Feb 22 14:38:06 2017 +0100
>
>   efi: create new early memory allocator

Does anybody know why this still fails? Andrew applied a fix (workaround) for 
this yesterday.

Daniel

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

Re: [Xen-devel] [PATCH v16 5/9] x86: change default load address from 1 MiB to 2 MiB

[Daniel Kiper]

On Wed, Mar 01, 2017 at 03:34:47AM -0700, Jan Beulich wrote:
> >>> On 01.03.17 at 11:13,  wrote:
> > On Wed, Mar 01, 2017 at 02:05:39AM -0700, Jan Beulich wrote:
> >> >>> On 21.02.17 at 20:19,  wrote:
> >> > Subsequent patches introducing relocatable early boot code play with
> >> > page tables using 2 MiB huge pages. If load address is not aligned at
> >> > 2 MiB then code touching such page tables must have special cases for
> >> > start and end of Xen image memory region. So, let's make life easier
> >> > and move default load address from 1 MiB to 2 MiB. This way page table
> >> > code will be nice and easy. Hence, there is a chance that it will be
> >> > less error prone too... :-)))
> >> >
> >> > Additionally, drop first 2 MiB mapping from Xen image mapping.
> >> > It is no longer needed.
> >>
> >> What about the memory allocated for it? Aiui at least in the xen.efi
> >> case there would be a 2Mb chunk left unused, but also never freed.
> >
> > Memory is not allocated for it.
>
> Are you sure? Where would the PE32+ header live in that case?

Is the PE32+ header loaded into memory? I think that only code and data stuff
is put there. Header is useful only for loader. Am I missing something?

Daniel

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