Re: [PATCH 4/5] multiboot2: parse console= option when setting GOP mode

[Daniel Kiper]

Sorry for late reply...

On Mon, Dec 05, 2022 at 04:10:28PM +0100, Jan Beulich wrote:
> On 23.11.2022 16:45, Roger Pau Monne wrote:
> > Only set the GOP mode if vga is selected in the console option,
> > otherwise just fetch the information from the current mode in order to
> > make it available to dom0.
> >
> > Introduce support for passing the command line to the efi_multiboot2()
> > helper, and parse the console= option if present.
> >
> > Signed-off-by: Roger Pau Monné 
> > ---
> > I'm unsure why the parsing of the multiboot2 tags is done in assembly,
> > it could very well be done in efi_multiboot2() in C, but I don't want
> > to switch that code now.
>
> I guess that's mainly mirroring the non-EFI boot path, where the amount
> of work needed to eventually enter C land is quite a bit larger?
> Anything beyond that Daniel may want to point out.

Yeah, you are right, its mainly mirroring the non-EFI boot path and it
evolved from that code. However, if you want to move it to C go for it...

Daniel

Re: [PATCH v5] templates: introduce GRUB_TOP_LEVEL_* vars

[Daniel Kiper]

On Mon, Oct 24, 2022 at 10:21:25PM -0500, Oskari Pirhonen wrote:
> On Mon, Oct 24, 2022 at 03:46:42 -0700, Denton Liu wrote:
> > A user may wish to use an image that is not sorted as the "latest"
> > version as the top-level entry. For example, in Arch Linux, if a user
> > has the LTS and regular kernels installed, `/boot/vmlinuz-linux-lts`
> > gets sorted as the "latest" compared to `/boot/vmlinuz-linux`, meaning
> > the LTS kernel becomes the top-level entry. However, a user may wish to
> > use the regular kernel as the top-level default with the LTS only
> > existing as a backup.
> >
> > This need can be seen in Arch Linux's AUR with two user-submitted
> > packages[0][1] providing an update hook which patches
> > /etc/grub.d/10_linux to move the desired kernel to the top-level. This
> > patch serves to solve this in a more generic way.
> >
> > Introduce the GRUB_TOP_LEVEL, GRUB_TOP_LEVEL_XEN and
> > GRUB_TOP_LEVEL_OS_PROBER variables to allow users to specify the
> > top-level entry.
> >
> > Create grub_move_to_front() as a helper function which moves entries to
> > the front of a list. This function does the heavy lifting of moving
> > the menu entry to the front in each script.
> >
> > In 10_netbsd, since there isn't an explicit list variable, extract the
> > items that are being iterated through into a list so that we can
> > optionally apply grub_move_to_front() to the list before the loop.
> >
> > [0]: https://aur.archlinux.org/packages/grub-linux-default-hook
> > [1]: https://aur.archlinux.org/packages/grub-linux-rt-default-hook
> >
> > Signed-off-by: Denton Liu 
>
> Reviewed-by: Oskari Pirhonen 

Reviewed-by: Daniel Kiper 

If I do not hear any objections in a week or so I will merge this patch.

Daniel

Re: [PATCH v3 2/5] xen/x86: manually build xen.mb.efi binary

[Daniel Kiper]

On Wed, May 19, 2021 at 04:35:00PM +0200, Jan Beulich wrote:
> On 19.05.2021 14:48, Daniel Kiper wrote:
> > On Wed, May 19, 2021 at 11:29:43AM +0200, Jan Beulich wrote:
> >> On 18.05.2021 19:46, Daniel Kiper wrote:
> >>> On Mon, May 17, 2021 at 03:24:28PM +0200, Jan Beulich wrote:
> >>>> On 17.05.2021 15:20, Daniel Kiper wrote:
> >>>>> On Mon, May 17, 2021 at 08:48:32AM +0200, Jan Beulich wrote:
> >>>>>> On 07.05.2021 22:26, Bob Eshleman wrote:
> >>>>>>> What is your intuition WRT the idea that instead of trying add a 
> >>>>>>> PE/COFF hdr
> >>>>>>> in front of Xen's mb2 bin, we instead go the route of introducing 
> >>>>>>> valid mb2
> >>>>>>> entry points into xen.efi?
> >>>>>>
> >>>>>> At the first glance I think this is going to be less intrusive, and 
> >>>>>> hence
> >>>>>> to be preferred. But of course I haven't experimented in any way ...
> >>>>>
> >>>>> When I worked on this a few years ago I tried that way. Sadly I failed
> >>>>> because I was not able to produce "linear" PE image using binutils
> >>>>> exiting that days.
> >>>>
> >>>> What is a "linear" PE image?
> >>>
> >>> The problem with Multiboot family protocols is that all code and data
> >>> sections have to be glued together in the image and as such loaded into
> >>> the memory (IIRC BSS is an exception but it has to live behind the
> >>> image). So, you cannot use PE image which has different representation
> >>> in file and memory. IIRC by default at least code and data sections in
> >>> xen.efi have different sizes in PE file and in memory. I tried to fix
> >>> that using linker script and objcopy but it did not work. Sadly I do
> >>> not remember the details but there is pretty good chance you can find
> >>> relevant emails in Xen-devel archive with me explaining what kind of
> >>> problems I met.
> >>
> >> Ah, this rings a bell. Even the .bss-is-last assumption doesn't hold,
> >> because .reloc (for us as well as in general) comes later, but needs
> >> loading (in the right place). Since even xen.gz isn't simply the
> >
> > However, IIRC it is not used when Xen is loaded through Multiboot2
> > protocol. So, I think it may stay in the image as is and the Mutliboot2
> > header should not cover .reloc section.
> >
> > By the way, why do we need .reloc section in the PE image? Is not %rip
> > relative addressing sufficient? IIRC the Linux kernel just contains
> > a stub .reloc section. Could not we do the same?
>
> %rip-relative addressing can (obviously, I think) help only for text.
> But we also have data containing pointers, which need relocating.

Ahhh, right, I totally forgot about it.

> >> compressed linker output, but a post-processed (by mkelf32) image,
> >> maybe what we need is a build tool doing similar post-processing on
> >> xen.efi? Otoh getting disk image and in-memory image aligned ought
> >
> > Yep, this should work too.
> >
> >> to be possible by setting --section-alignment= and --file-alignment=
> >> to the same value (resulting in a much larger file) - adjusting file
> >
> > IIRC this did not work for some reason. Maybe it would be better to
> > enforce correct alignment and required padding using linker script.
>
> I'm not convinced the linker script is the correct vehicle here. It
> is mainly about placement in the address space (i.e. laying out how
> things will end up in memory), not about file layout.

OK but at least I would check what is possible and do it then.

> >> positions would effectively be what a post-processing tool would need
> >> to do (like with mkelf32 perhaps we could then at least save the
> >> first ~2Mb of space). Which would still leave .reloc to be dealt with
> >> - maybe we could place this after .init, but still ahead of
> >> __init_end (such that the memory would get freed late in the boot
> >> process). Not sure whether EFI loaders would "like" such an unusual
> >> placement.
> >
> > Yeah, good question...
> >
> >> Also not sure what to do with Dwarf debug info, which just recently
> >> we managed to avoid needing to strip unconditionally.
> >
> > I think debug info may stay as is. Just Multiboot2 header should not
> > cover it if it is not needed.
>
> You did say that .bss is expected to be last, which both .reloc and
> debug info violate.

The .bss section has to be last one in memory from Multiboot2 protocol
point of view. However, nothing, AFAICT, forbids to have something
behind in the file. Of course if you ignore the data at the end of file
when you load the image using Multiboot2 protocol.

Daniel

Re: [PATCH v3 2/5] xen/x86: manually build xen.mb.efi binary

[Daniel Kiper]

On Wed, May 19, 2021 at 11:29:43AM +0200, Jan Beulich wrote:
> On 18.05.2021 19:46, Daniel Kiper wrote:
> > On Mon, May 17, 2021 at 03:24:28PM +0200, Jan Beulich wrote:
> >> On 17.05.2021 15:20, Daniel Kiper wrote:
> >>> On Mon, May 17, 2021 at 08:48:32AM +0200, Jan Beulich wrote:
> >>>> On 07.05.2021 22:26, Bob Eshleman wrote:
> >>>>> What is your intuition WRT the idea that instead of trying add a 
> >>>>> PE/COFF hdr
> >>>>> in front of Xen's mb2 bin, we instead go the route of introducing valid 
> >>>>> mb2
> >>>>> entry points into xen.efi?
> >>>>
> >>>> At the first glance I think this is going to be less intrusive, and hence
> >>>> to be preferred. But of course I haven't experimented in any way ...
> >>>
> >>> When I worked on this a few years ago I tried that way. Sadly I failed
> >>> because I was not able to produce "linear" PE image using binutils
> >>> exiting that days.
> >>
> >> What is a "linear" PE image?
> >
> > The problem with Multiboot family protocols is that all code and data
> > sections have to be glued together in the image and as such loaded into
> > the memory (IIRC BSS is an exception but it has to live behind the
> > image). So, you cannot use PE image which has different representation
> > in file and memory. IIRC by default at least code and data sections in
> > xen.efi have different sizes in PE file and in memory. I tried to fix
> > that using linker script and objcopy but it did not work. Sadly I do
> > not remember the details but there is pretty good chance you can find
> > relevant emails in Xen-devel archive with me explaining what kind of
> > problems I met.
>
> Ah, this rings a bell. Even the .bss-is-last assumption doesn't hold,
> because .reloc (for us as well as in general) comes later, but needs
> loading (in the right place). Since even xen.gz isn't simply the

However, IIRC it is not used when Xen is loaded through Multiboot2
protocol. So, I think it may stay in the image as is and the Mutliboot2
header should not cover .reloc section.

By the way, why do we need .reloc section in the PE image? Is not %rip
relative addressing sufficient? IIRC the Linux kernel just contains
a stub .reloc section. Could not we do the same?

> compressed linker output, but a post-processed (by mkelf32) image,
> maybe what we need is a build tool doing similar post-processing on
> xen.efi? Otoh getting disk image and in-memory image aligned ought

Yep, this should work too.

> to be possible by setting --section-alignment= and --file-alignment=
> to the same value (resulting in a much larger file) - adjusting file

IIRC this did not work for some reason. Maybe it would be better to
enforce correct alignment and required padding using linker script.

> positions would effectively be what a post-processing tool would need
> to do (like with mkelf32 perhaps we could then at least save the
> first ~2Mb of space). Which would still leave .reloc to be dealt with
> - maybe we could place this after .init, but still ahead of
> __init_end (such that the memory would get freed late in the boot
> process). Not sure whether EFI loaders would "like" such an unusual
> placement.

Yeah, good question...

> Also not sure what to do with Dwarf debug info, which just recently
> we managed to avoid needing to strip unconditionally.

I think debug info may stay as is. Just Multiboot2 header should not
cover it if it is not needed.

> >>> Maybe
> >>> newer binutils are more flexible and will be able to produce a PE image
> >>> with properties required by Multiboot2 protocol.
> >>
> >> Isn't all you need the MB2 header within the first so many bytes of the
> >> (disk) image? Or was it the image as loaded into memory? Both should be
> >> possible to arrange for.
> >
> > IIRC Multiboot2 protocol requires the header in first 32 kiB of an image.
> > So, this is not a problem.
>
> I was about to ask "Disk image or in-memory image?" But this won't
> matter if the image as a whole got linearized, as long as the first
> section doesn't start to high up. I notice that xen-syms doesn't fit
> this requirement either, only the output of mkelf32 does. Which
> suggests that there may not be a way around a post-processing tool.

Could not we drop 2 MiB padding at the beginning of xen-syms by changing
some things in the linker script?

Daniel

Re: [PATCH v3 2/5] xen/x86: manually build xen.mb.efi binary

[Daniel Kiper]

On Mon, May 17, 2021 at 03:24:28PM +0200, Jan Beulich wrote:
> On 17.05.2021 15:20, Daniel Kiper wrote:
> > On Mon, May 17, 2021 at 08:48:32AM +0200, Jan Beulich wrote:
> >> On 07.05.2021 22:26, Bob Eshleman wrote:
> >>> What is your intuition WRT the idea that instead of trying add a PE/COFF 
> >>> hdr
> >>> in front of Xen's mb2 bin, we instead go the route of introducing valid 
> >>> mb2
> >>> entry points into xen.efi?
> >>
> >> At the first glance I think this is going to be less intrusive, and hence
> >> to be preferred. But of course I haven't experimented in any way ...
> >
> > When I worked on this a few years ago I tried that way. Sadly I failed
> > because I was not able to produce "linear" PE image using binutils
> > exiting that days.
>
> What is a "linear" PE image?

The problem with Multiboot family protocols is that all code and data
sections have to be glued together in the image and as such loaded into
the memory (IIRC BSS is an exception but it has to live behind the
image). So, you cannot use PE image which has different representation
in file and memory. IIRC by default at least code and data sections in
xen.efi have different sizes in PE file and in memory. I tried to fix
that using linker script and objcopy but it did not work. Sadly I do
not remember the details but there is pretty good chance you can find
relevant emails in Xen-devel archive with me explaining what kind of
problems I met.

> > Maybe
> > newer binutils are more flexible and will be able to produce a PE image
> > with properties required by Multiboot2 protocol.
>
> Isn't all you need the MB2 header within the first so many bytes of the
> (disk) image? Or was it the image as loaded into memory? Both should be
> possible to arrange for.

IIRC Multiboot2 protocol requires the header in first 32 kiB of an image.
So, this is not a problem.

Daniel

Re: [PATCH v3 2/5] xen/x86: manually build xen.mb.efi binary

[Daniel Kiper]

On Mon, May 17, 2021 at 08:48:32AM +0200, Jan Beulich wrote:
> On 07.05.2021 22:26, Bob Eshleman wrote:
> > What is your intuition WRT the idea that instead of trying add a PE/COFF hdr
> > in front of Xen's mb2 bin, we instead go the route of introducing valid mb2
> > entry points into xen.efi?
>
> At the first glance I think this is going to be less intrusive, and hence
> to be preferred. But of course I haven't experimented in any way ...

When I worked on this a few years ago I tried that way. Sadly I failed
because I was not able to produce "linear" PE image using binutils
exiting that days. Though I think you should try once again. Maybe
newer binutils are more flexible and will be able to produce a PE image
with properties required by Multiboot2 protocol.

Daniel

Re: multiboot2 and module2 boot issues via GRUB2

[Daniel Kiper]

On Thu, Apr 01, 2021 at 08:43:46PM +0100, Andrew Cooper via Grub-devel wrote:
> On 01/04/2021 09:44, Roger Pau Monné wrote:
> > On Thu, Apr 01, 2021 at 09:31:07AM +0200, Jan Beulich wrote:
> >> On 01.04.2021 03:06, Roman Shaposhnik wrote:
> >>> And the obvious next question: is my EVE usecase esoteric enough that
> >>> I should just go ahead and do a custom GRUB patch or is there a more
> >>> general interest in this?
> >> Not sure if it ought to be a grub patch - the issue could as well
> >> be dealt with in Xen, by concatenating modules to form a monolithic
> >> initrd.
> > I would rather have it done in the loader than Xen, mostly because
> > it's a Linux boot specific format, and hence I don't think Xen should
> > have any knowledge about it.
> >
> > If it turns out to be impossible to implement on the loader side we
> > should consider doing it in Xen, but that's not my first option.
>
> Concatenating random things which may or may not be initrds is
> absolutely not something Xen should do.  We don't have enough context to
> do it safely/sensibly.
>
> Honestly, I like the idea of supporting something like this generally in
> grub.  Linux already commonly has initrd preparation prepending an
> uncompressed microcode CPIO archive, and I can see a usability advantage
> from maintaining the initrd fragments separately.
>
> Looking at the grub manual, this behaviour of the `initrd` command isn't
> even documented.  Perhaps that should be fixed first, and then maybe
> `module2_multi` added too?

I am OK with additional Multiboot2 command. Though I would do
s/module2_multi/module2_concat/. Additionally, it should look for "--"
and interpret everything after it as a command line argument for
a concatenated module.

Daniel

[SPECIFICATION RFC] The firmware and bootloader log specification

[Daniel Kiper]

Hey,

This is next attempt to create firmware and bootloader log specification.
Due to high interest among industry it is an extension to the initial
bootloader log only specification. It takes into the account most of the
comments which I got up until now.

The goal is to pass all logs produced by various boot components to the
running OS. The OS kernel should expose these logs to the user space
and/or process them internally if needed. The content of these logs
should be human readable. However, they should also contain the
information which allows admins to do e.g. boot time analysis.

The log specification should be as much as possible platform agnostic
and self contained. The final version of this spec should be merged into
existing specifications, e.g. UEFI, ACPI, Multiboot2, or be a standalone
spec, e.g. as a part of OASIS Standards. The former seems better but is
not perfect too...

Here is the description (pseudocode) of the structures which will be
used to store the log data.

  struct bf_log
  {
uint32_t   version;
char   producer[64];
uint64_t   flags;
uint64_t   next_bf_log_addr;
uint32_t   next_msg_off;
bf_log_msg msgs[];
  }

  struct bf_log_msg
  {
uint32_t size;
uint64_t ts_nsec;
uint32_t level;
uint32_t facility;
uint32_t msg_off;
char strings[];
  }

The members of struct bf_log:
  - version: the firmware and bootloader log format version number, 1 for now,
  - producer: the producer/firmware/bootloader/... type; the length
allows ASCII UUID storage if somebody needs that functionality,
  - flags: it can be used to store information about log state, e.g.
it was truncated or not (does it make sense to have an information
about the number of lost messages?),
  - next_bf_log_addr: address of next bf_log struct; none if zero (I think
newer spec versions should not change anything in first 5 bf_log members;
this way older log parsers will be able to traverse/copy all logs regardless
of version used in one log or another),
  - next_msg_off: the offset, in bytes, from the beginning of the bf_log struct,
of the next byte after the last log message in the msgs[]; i.e. the offset
of the next available log message slot; it is equal to the total size of
the log buffer including the bf_log struct,
  - msgs: the array of log messages,
  - should we add CRC or hash or signatures here?

The members of struct bf_log_msg:
  - size: total size of bf_log_msg struct,
  - ts_nsec: timestamp expressed in nanoseconds starting from 0,
  - level: similar to syslog meaning; can be used to differentiate normal 
messages
from debug messages; the exact interpretation depends on the current 
producer
type specified in the bf_log.producer,
  - facility: similar to syslog meaning; can be used to differentiate the 
sources of
the messages, e.g. message produced by networking module; the exact 
interpretation
depends on the current producer type specified in the bf_log.producer,
  - msg_off: the log message offset in strings[],
  - strings[0]: the beginning of log message type, similar to the facility 
member but
NUL terminated string instead of integer; this will be used by, e.g., the 
GRUB2
for messages printed using grub_dprintf(),
  - strings[msg_off]: the beginning of log message, NUL terminated string.

Note: The producers are free to use/ignore any given set of level, facility 
and/or
  log type members. Though the usage of these members has to be clearly 
defined.
  Ignored integer members should be set to 0. Ignored log message type 
should
  contain an empty NUL terminated string. The log message is mandatory but 
can
  be an empty NUL terminated string.

There is still not fully solved problem how the logs should be presented to the 
OS.
On the UEFI platforms we can use config tables to do that. Then probably
bf_log.next_bf_log_addr should not be used. On the ACPI and Device Tree 
platforms
we can use these mechanisms to present the logs to the OSes. The situation gets 
more
difficult if neither of these mechanisms are present. However, maybe we should 
not
bother too much about that because probably these platforms getting less and 
less
common.

Anyway, I am aware that this is not specification per se. The goal of this 
email is
to continue the discussion about the idea of the firmware and booloader log and 
to
find out where the final specification should land. Of course taking into the 
account
assumptions made above.

You can find previous discussions about related topics at [1], [2] and [3].

Additionally, I am going to present this during GRUB mini-summit session on 
Tuesday,
17th of November at 15:45 UTC. So, if you want to discuss the log design please 
join
us. You can find more details here [4].

Daniel

[1] https://lists.gnu.org/archive/html/grub-devel/2019-10/msg00107.html
[2] https://lists.gnu.org/archive/html/grub-devel/2019-11/msg00079.html
[3] 

Re: [BOOTLOADER SPECIFICATION RFC] The bootloader log format for TrenchBoot and others

[Daniel Kiper]

On Fri, May 29, 2020 at 10:11:40AM -0700, Andy Lutomirski wrote:
> > On May 29, 2020, at 4:30 AM, Daniel Kiper  wrote:
> >
> > Hey,
> >
> > Below you can find my rough idea of the bootloader log format which is
> > generic thing but initially will be used for TrenchBoot work. I discussed
> > this proposal with Ross and Daniel S. So, the idea went through initial
> > sanitization. Now I would like to take feedback from other folks too.
> > So, please take a look and complain...
>
> > In general we want to pass the messages produced by the bootloader to the OS
> > kernel and finally to the userspace for further processing and analysis. 
> > Below
> > is the description of the structures which will be used for this thing.
>
> Is the intent for a human to read these, or to get them into the
> system log file, or are they intended to actually change the behavior
> of the system?
>
> IOW, what is this for?

In general the idea is for a human to read these. There will be a separate
tool which reads the bootloader log exposed via Linux kernel sysfs and displays
its contents to the user. The tool will allow user to do various kinds of
filtering. We are not going to put the contents of the bootloader log into any
of system logs. However, if somebody wants to do that, in sensible way, I am
OK with that.

Daniel

[BOOTLOADER SPECIFICATION RFC] The bootloader log format for TrenchBoot and others

[Daniel Kiper]

Hey,

Below you can find my rough idea of the bootloader log format which is
generic thing but initially will be used for TrenchBoot work. I discussed
this proposal with Ross and Daniel S. So, the idea went through initial
sanitization. Now I would like to take feedback from other folks too.
So, please take a look and complain...

In general we want to pass the messages produced by the bootloader to the OS
kernel and finally to the userspace for further processing and analysis. Below
is the description of the structures which will be used for this thing.

  struct bootloader_log_msgs
  {
grub_uint32_t level;
grub_uint32_t facility;
char type[];
char msg[];
  }

  struct bootloader_log
  {
grub_uint32_t version;
grub_uint32_t producer;
grub_uint32_t size;
grub_uint32_t next_off;
bootloader_log_msgs msgs[];
  }

The members of struct bootloader_log:
  - version: the bootloader log format version number, 1 for now,
  - producer: the producer/bootloader type; we can steal some values from
linux/Documentation/x86/boot.rst:type_of_loader,
  - size: total size of the log buffer including the bootloader_log struct,
  - next_off: offset in bytes, from start of the bootloader_log struct,
of the next byte after the last log message in the msgs[];
i.e. the offset of the next available log message slot,
  - msgs: the array of log messages.

The members of struct bootloader_log_msgs:
  - level: similar to syslog meaning; can be used to differentiate
normal messages from debug messages; exact interpretation depends
on the current producer/bootloader type specified in the
bootloader_log.producer,
  - facility: similar to syslog meaning; can be used to differentiate
the sources of the messages, e.g. message produced by networking
module; exact interpretation depends on the current producer/bootloader
type specified in the bootloader_log.producer,
  - type: similar to the facility member but NUL terminated string instead of 
integer;
this will be used by GRUB for messages printed using grub_dprintf(),
  - msg: the bootloader log message, NUL terminated string.

Note: The bootloaders are free to use/ignore any given set of level,
  facility and/or type members. Though the usage of these members
  has to be clearly defined. Ignored integer members should be set
  to 0. Ignored type member should contain an empty NUL terminated
  string. msg member is mandatory but can be an empty NUL terminated
  string.

Taking into account [1] and [2] I want to make this functionality generic
as much as possible. So, this bootloader log can be used with any bootloader
and OS kernel. However, initially the functionality will be implemented for
the Linux kernel and its boot protocol.

In case of Linux kernel the pointer to the bootloader_log struct should
be passed from the bootloader to the kernel through the boot_params and
the bootloader_log struct contents should be exposed via sysfs. E.g.
somewhere at /sys/kernel/debug or /sys/kernel/tracing or maybe we should
create new /sys/bootloader/log node.

If everybody is OK with this rough proposal then I will start working
on making it a part of Multiboot2 specification (the text above is just
raw description of the idea; it is not final text which land into the
spec). If you see better place for this thing just drop me a line.

Daniel

[1] https://lists.gnu.org/archive/html/grub-devel/2019-10/msg00107.html
[2] https://lists.gnu.org/archive/html/grub-devel/2019-11/msg00079.html

Re: [GRUB PATCH 0/2] Better Xen support

[Daniel Kiper]

On Wed, May 20, 2020 at 01:14:18PM +0100, Ian Jackson wrote:
> Hi. As maintainer of the Xen Project upstream CI, I do testing of
> upstream Xen builds onto Debian systems.
>
> We use grub's 20_linux_xen to do the bootloader setup.  However, it is
> missing some features so we are carrying some patches.  Here they are
> for your consideration.
>
> Regards, Ian.
>
> Ian Jackson (2):
>   20_linux_xen: Ignore xenpolicy and config files too
>   20_linux_xen: Support Xen Security Modules (XSM/FLASK)

Reviewed-by: Daniel Kiper 

Daniel

Re: [RFC] UEFI Secure Boot on Xen Hosts

[Daniel Kiper]

On Fri, May 01, 2020 at 12:27:17AM +0200, Marek Marczykowski-Górecki wrote:
> On Wed, Apr 29, 2020 at 05:51:08PM -0500, Bobby Eshleman wrote:
> > # Option #3: Lean on Grub2's LoadFile2() Verification
> >
> > Grub2 will provide a LoadFile2() method to subsequent programs that supports
> > signature verification of arbitrary files.  Linux is moving in the
> > direction of using LoadFile2() for loading the initrd [2], and Grub2 will
> > support verifying the signatures of files loaded via LoadFile2().
>
> This description gives me flashbacks to how xen.efi loads dom0
> kernel+initramfs (Loaded Image Protocol). Practical issues encountered:
>
> 1. It works only when xen.efi is loaded via appropriate EFI boot
> service directly. If xen.efi is loaded by a filesystem driver in
> grub/other bootloader, it can't load dom0 kernel+initramfs.
>
> 2. Given variety of UEFI implementations and boot medias, it was almost
> impossible to force bootloader to use the right file loading method
> (cases like the same file accessible both via UEFI fs0: and via grub's
> filesystem driver). Which means loading xen.efi via a bootloader (as
> opposed to directly from UEFI) was hit or miss (mostly miss).
>
> 3. To avoid the above issue, one was forced to load xen.efi directly
> from EFI. This gave up any possibility to manipulate xen cmdline, or
> even choose system to boot in case of some EFI implementations.

Are you talking about GRUB chainloader command? If yes then use "set root=..."
to select ESP before running the chainloader. Of course the xen.cfg,
kernel and initramfs have to live on ESP then. Xen uses UEFI file system
calls which understand FAT only. And if GRUB root points to non-FAT
partition than Xen cannot read anything from it...

> 4. Even if one is lucky to boot xen.efi via grub2-efi, then still only
> xen options could be modified. But not those for dom0.
>
> 5. It was impossible to load xen/kernel/initrd using fancy grub/ipxe
> features like HTTP.

Why cannot you use multiboot2/module2 to load Xen from GRUB on x86 UEFI
machines? It does not have issues discussed above. Additionally, the
Multiboot2 protocol works on legacy BIOS machines too.

> In practice the above points mean:
>
> * To get a usable product, one is forced to enable all kind of
>   workarounds (not only related to EFI) _in default configuration_,
>   because otherwise there is very little chance to enable them only when
>   needed.
>
> * If something goes wrong, in most cases you need to boot from
>   alternative media (to edit xen.cfg, or efi variables). If you happen
>   to forget your rescue usb stick, you are out of luck.
>
> So, please, can someone confirm the LoadFile2() approach wouldn't have
> any of the above issues?

If it is done properly it should not.

Daniel

Re: [RFC] UEFI Secure Boot on Xen Hosts

[Daniel Kiper]

Adding Ard...

On Thu, Apr 30, 2020 at 09:01:45AM +0200, Jan Beulich wrote:
> On 30.04.2020 00:51, Bobby Eshleman wrote:
> > Hey all,
> >
> > We're looking to develop UEFI Secure Boot support for grub-loaded Xen and
> > ultimately for XCP-ng (I'm on the XCP-ng team at Vates).
> >
> > In addition to carrying the chain-of-trust through the entire boot sequence
> > into dom0, we would also like to build something akin to Linux's Lockdown 
> > for
> > dom0 and its privileged interfaces.
> >
> > It appears that there are various options and we'd like to discuss them with
> > the community.
> >
> > # Option #1: PE/COFF and Shim
> >
> > Shim installs a verification protocol available to subsequent programs via 
> > EFI
> > boot services.  The protocol is called SHIM_LOCK and it is currently 
> > supported
> > by xen.efi.
> >
> > Shim requires the payload under verification to be a PE/COFF executable.  In
> > order to support both shim and maintain the multiboot2 protocol, Daniel 
> > Kiper's
> > patchset [1]  (among other things) incorporates the PE/COFF header
> > into xen.gz and adds dom0 verification via SHIM_LOCK in
> > efi_multiboot2().
> >
> > There appears that some work will be needed on top of this patchset, but not
> > much as it seems most of the foot work has been done.
> >
> > AFAIK, the changes needed in GRUB for this approach are already upstream 
> > (the
> > shim_lock module is upstream), and shim would go untouched.
> >
> > # Option #2: Extended Multiboot2 and Shim
> >
> > Another approach that could be taken is to embed Xen's signature into a
> > new multiboot2 header and then modify shim to support it.  This would
> > arguably be more readable than embedding the PE/COFF header, would add
> > value to shim, and would fit nicely with the mb2 header code that
> > already exists in Xen.  The downside being that it would require a shim
> > fork.  Grub2 would be unchanged.

Here you have to change the Multiboot2 protocol and singing tools too.
So, I do not consider this as a viable solution.

> > I'm not familliar with Microsoft's signing process.  I do know they
> > support template submissions based on shim, and I'm not sure if such a
> > big change would impact their approval process.
> >
> > # Option #3: Lean on Grub2's LoadFile2() Verification
> >
> > Grub2 will provide a LoadFile2() method to subsequent programs that supports
> > signature verification of arbitrary files.  Linux is moving in the
> > direction of using LoadFile2() for loading the initrd [2], and Grub2 will
> > support verifying the signatures of files loaded via LoadFile2().  This is 
> > set
> > for release in GRUB 2.06 sometime in the latter half of 2020.

s/for release in/after release/

> > In Xen, this approach could be used for loading dom0 as well, offering a 
> > very
> > simple verified load interface.
> >
> > Currently the initrd argument passed from Linux to LoadFile2() is a vendor
> > media device path GUID [3].
> >
> > Changes to Xen:
> > - Xen would need to pass these device paths to Grub
> > - Xen would be changed to load dom0 with the LoadFile2() interface via boot 
> > services
> >
> > Changes to Grub:
> > - Xen dom0 kernel/initrd device paths would need to be introduced to Grub

Maybe partially but certainly there will be some differences...

> > Potential Issues:
> > - How will Xen handle more boot modules than just a dom0 and dom0
> >   initrd?
> > - Would each boot module need a unique vendor guid?

AIUI yes but Ard, who designed this new boot protocol, may say more
about that.

Anyway, the advantage of this new protocol is that it uses UEFI API to
load and execute PE kernel and its modules. This means that it will be
architecture independent. However, IIRC, if we want to add new modules
types to the Xen then we have to teach all bootloaders in the wild about
new GUIDs. Ard, am I correct?

> > - Would this interfere with the DomB proposal?  I suspect not because
> >   the DomB proposal suggests launching DomUs from an already booted
> >   DomB, at which point other means could be used.
> >
> > We'd just like to get the conversation going on this topic before we
> > dive too far into implementing something.  Are any of these approaches a
> > hard no for upstreaming?  Do any stand out as best candidates?  Any
> > feedback / questions / criticisms would be greatly appreciated.
>
> A shim fork doesn't look desirable, which would rule out #2 unless there
> is an option there to avoid the fork.
>
> If the potential issues listed for #3 can be suitably addressed, I can't
> currently see a reason to prefer either of the two remaining options; I
> vaguely recall though that Daniel's change for #1 didn't look overly
> appealing, but perhaps this can be taken care of.

Daniel

[Xen-devel] [tip: x86/boot] x86/boot: Introduce kernel_info.setup_type_max

[tip-bot2 for Daniel Kiper]

The following commit has been merged into the x86/boot branch of tip:

Commit-ID: 00cd1c154d565c62ad5e065bf3530f68bdf59490
Gitweb:
https://git.kernel.org/tip/00cd1c154d565c62ad5e065bf3530f68bdf59490
Author:Daniel Kiper 
AuthorDate:Tue, 12 Nov 2019 14:46:39 +01:00
Committer: Borislav Petkov 
CommitterDate: Tue, 12 Nov 2019 16:16:54 +01:00

x86/boot: Introduce kernel_info.setup_type_max

This field contains maximal allowed type for setup_data.

Do not bump setup_header version in arch/x86/boot/header.S because it
will be followed by additional changes coming into the Linux/x86 boot
protocol.

Suggested-by: H. Peter Anvin (Intel) 
Signed-off-by: Daniel Kiper 
Signed-off-by: Borislav Petkov 
Reviewed-by: Konrad Rzeszutek Wilk 
Reviewed-by: Ross Philipson 
Reviewed-by: H. Peter Anvin (Intel) 
Cc: Andy Lutomirski 
Cc: ard.biesheu...@linaro.org
Cc: Boris Ostrovsky 
Cc: dave.han...@linux.intel.com
Cc: eric.snowb...@oracle.com
Cc: Ingo Molnar 
Cc: Jonathan Corbet 
Cc: Juergen Gross 
Cc: kanth.ghatr...@oracle.com
Cc: linux-...@vger.kernel.org
Cc: linux-efi 
Cc: Peter Zijlstra 
Cc: rdun...@infradead.org
Cc: ross.philip...@oracle.com
Cc: Thomas Gleixner 
Cc: x86-ml 
Cc: xen-devel@lists.xenproject.org
Link: https://lkml.kernel.org/r/20191112134640.16035-3-daniel.ki...@oracle.com
---
 Documentation/x86/boot.rst |  9 -
 arch/x86/boot/compressed/kernel_info.S |  5 +
 arch/x86/include/uapi/asm/bootparam.h  |  3 +++
 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/Documentation/x86/boot.rst b/Documentation/x86/boot.rst
index c60fafd..6cdd767 100644
--- a/Documentation/x86/boot.rst
+++ b/Documentation/x86/boot.rst
@@ -73,7 +73,7 @@ Protocol 2.14:BURNT BY INCORRECT COMMIT 
ae7e1238e68f2a472a125673ab506d49158c188
(x86/boot: Add ACPI RSDP address to setup_header)
DO NOT USE!!! ASSUME SAME AS 2.13.
 
-Protocol 2.15: (Kernel 5.5) Added the kernel_info.
+Protocol 2.15: (Kernel 5.5) Added the kernel_info and 
kernel_info.setup_type_max.
 =  
 
 .. note::
@@ -981,6 +981,13 @@ Offset/size:   0x0008/4
   This field contains the size of the kernel_info including kernel_info.header
   and kernel_info.kernel_info_var_len_data.
 
+   ==
+Field name:setup_type_max
+Offset/size:   0x000c/4
+   ==
+
+  This field contains maximal allowed type for setup_data.
+
 
 The Image Checksum
 ==
diff --git a/arch/x86/boot/compressed/kernel_info.S 
b/arch/x86/boot/compressed/kernel_info.S
index 8ea6f6e..018dacb 100644
--- a/arch/x86/boot/compressed/kernel_info.S
+++ b/arch/x86/boot/compressed/kernel_info.S
@@ -1,5 +1,7 @@
 /* SPDX-License-Identifier: GPL-2.0 */
 
+#include 
+
.section ".rodata.kernel_info", "a"
 
.global kernel_info
@@ -12,6 +14,9 @@ kernel_info:
/* Size total. */
.long   kernel_info_end - kernel_info
 
+   /* Maximal allowed type for setup_data. */
+   .long   SETUP_TYPE_MAX
+
 kernel_info_var_len_data:
/* Empty for time being... */
 kernel_info_end:
diff --git a/arch/x86/include/uapi/asm/bootparam.h 
b/arch/x86/include/uapi/asm/bootparam.h
index a1ebcd7..dbb4112 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -11,6 +11,9 @@
 #define SETUP_APPLE_PROPERTIES 5
 #define SETUP_JAILHOUSE6
 
+/* max(SETUP_*) */
+#define SETUP_TYPE_MAX SETUP_JAILHOUSE
+
 /* ram_size flags */
 #define RAMDISK_IMAGE_START_MASK   0x07FF
 #define RAMDISK_PROMPT_FLAG0x8000

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

[Xen-devel] [tip: x86/boot] x86/boot: Introduce kernel_info

[tip-bot2 for Daniel Kiper]

The following commit has been merged into the x86/boot branch of tip:

Commit-ID: 2c33c27fd6033ced942c9a591b8ac15c07c57d70
Gitweb:
https://git.kernel.org/tip/2c33c27fd6033ced942c9a591b8ac15c07c57d70
Author:Daniel Kiper 
AuthorDate:Tue, 12 Nov 2019 14:46:38 +01:00
Committer: Borislav Petkov 
CommitterDate: Tue, 12 Nov 2019 16:10:34 +01:00

x86/boot: Introduce kernel_info

The relationships between the headers are analogous to the various data
sections:

  setup_header = .data
  boot_params/setup_data = .bss

What is missing from the above list? That's right:

  kernel_info = .rodata

We have been (ab)using .data for things that could go into .rodata or .bss for
a long time, for lack of alternatives and -- especially early on -- inertia.
Also, the BIOS stub is responsible for creating boot_params, so it isn't
available to a BIOS-based loader (setup_data is, though).

setup_header is permanently limited to 144 bytes due to the reach of the
2-byte jump field, which doubles as a length field for the structure, combined
with the size of the "hole" in struct boot_params that a protected-mode loader
or the BIOS stub has to copy it into. It is currently 119 bytes long, which
leaves us with 25 very precious bytes. This isn't something that can be fixed
without revising the boot protocol entirely, breaking backwards compatibility.

boot_params proper is limited to 4096 bytes, but can be arbitrarily extended
by adding setup_data entries. It cannot be used to communicate properties of
the kernel image, because it is .bss and has no image-provided content.

kernel_info solves this by providing an extensible place for information about
the kernel image. It is readonly, because the kernel cannot rely on a
bootloader copying its contents anywhere, but that is OK; if it becomes
necessary it can still contain data items that an enabled bootloader would be
expected to copy into a setup_data chunk.

Do not bump setup_header version in arch/x86/boot/header.S because it
will be followed by additional changes coming into the Linux/x86 boot
protocol.

Suggested-by: H. Peter Anvin (Intel) 
Signed-off-by: Daniel Kiper 
Signed-off-by: Borislav Petkov 
Reviewed-by: Konrad Rzeszutek Wilk 
Reviewed-by: Ross Philipson 
Reviewed-by: H. Peter Anvin (Intel) 
Cc: Andy Lutomirski 
Cc: ard.biesheu...@linaro.org
Cc: Boris Ostrovsky 
Cc: dave.han...@linux.intel.com
Cc: eric.snowb...@oracle.com
Cc: Ingo Molnar 
Cc: Jonathan Corbet 
Cc: Juergen Gross 
Cc: kanth.ghatr...@oracle.com
Cc: linux-...@vger.kernel.org
Cc: linux-efi 
Cc: Peter Zijlstra 
Cc: rdun...@infradead.org
Cc: ross.philip...@oracle.com
Cc: Thomas Gleixner 
Cc: x86-ml 
Cc: xen-devel@lists.xenproject.org
Link: https://lkml.kernel.org/r/20191112134640.16035-2-daniel.ki...@oracle.com
---
 Documentation/x86/boot.rst | 126 -
 arch/x86/boot/Makefile |   2 +-
 arch/x86/boot/compressed/Makefile  |   4 +-
 arch/x86/boot/compressed/kernel_info.S |  17 +++-
 arch/x86/boot/header.S |   1 +-
 arch/x86/boot/tools/build.c|   5 +-
 arch/x86/include/uapi/asm/bootparam.h  |   1 +-
 7 files changed, 153 insertions(+), 3 deletions(-)
 create mode 100644 arch/x86/boot/compressed/kernel_info.S

diff --git a/Documentation/x86/boot.rst b/Documentation/x86/boot.rst
index 08a2f10..c60fafd 100644
--- a/Documentation/x86/boot.rst
+++ b/Documentation/x86/boot.rst
@@ -68,8 +68,25 @@ Protocol 2.12(Kernel 3.8) Added the xloadflags field 
and extension fields
 Protocol 2.13  (Kernel 3.14) Support 32- and 64-bit flags being set in
xloadflags to support booting a 64-bit kernel from 32-bit
EFI
+
+Protocol 2.14: BURNT BY INCORRECT COMMIT 
ae7e1238e68f2a472a125673ab506d49158c1889
+   (x86/boot: Add ACPI RSDP address to setup_header)
+   DO NOT USE!!! ASSUME SAME AS 2.13.
+
+Protocol 2.15: (Kernel 5.5) Added the kernel_info.
 =  
 
+.. note::
+ The protocol version number should be changed only if the setup header
+ is changed. There is no need to update the version number if boot_params
+ or kernel_info are changed. Additionally, it is recommended to use
+ xloadflags (in this case the protocol version number should not be
+ updated either) or kernel_info to communicate supported Linux kernel
+ features to the boot loader. Due to very limited space available in
+ the original setup header every update to it should be considered
+ with great care. Starting from the protocol 2.15 the primary way to
+ communicate things to the boot loader is the kernel_info.
+
 
 Memory Layout
 =
@@ -207,6 +224,7 @@ Offset/Size Proto   NameMeaning
 0258/8 2.10+   pref_addressPreferred loading 
address
 0260/4 2.10+   init_size   Linear memory requir

[Xen-devel] [tip: x86/boot] x86/boot: Introduce setup_indirect

[tip-bot2 for Daniel Kiper]

The following commit has been merged into the x86/boot branch of tip:

Commit-ID: b3c72fc9a78e74161f9d05ef7191706060628f8c
Gitweb:
https://git.kernel.org/tip/b3c72fc9a78e74161f9d05ef7191706060628f8c
Author:Daniel Kiper 
AuthorDate:Tue, 12 Nov 2019 14:46:40 +01:00
Committer: Borislav Petkov 
CommitterDate: Tue, 12 Nov 2019 16:21:15 +01:00

x86/boot: Introduce setup_indirect

The setup_data is a bit awkward to use for extremely large data objects,
both because the setup_data header has to be adjacent to the data object
and because it has a 32-bit length field. However, it is important that
intermediate stages of the boot process have a way to identify which
chunks of memory are occupied by kernel data. Thus introduce an uniform
way to specify such indirect data as setup_indirect struct and
SETUP_INDIRECT type.

And finally bump setup_header version in arch/x86/boot/header.S.

Suggested-by: H. Peter Anvin (Intel) 
Signed-off-by: Daniel Kiper 
Signed-off-by: Borislav Petkov 
Reviewed-by: Ross Philipson 
Reviewed-by: H. Peter Anvin (Intel) 
Acked-by: Konrad Rzeszutek Wilk 
Cc: Andy Lutomirski 
Cc: ard.biesheu...@linaro.org
Cc: Boris Ostrovsky 
Cc: dave.han...@linux.intel.com
Cc: eric.snowb...@oracle.com
Cc: Ingo Molnar 
Cc: Jonathan Corbet 
Cc: Juergen Gross 
Cc: kanth.ghatr...@oracle.com
Cc: linux-...@vger.kernel.org
Cc: linux-efi 
Cc: Peter Zijlstra 
Cc: rdun...@infradead.org
Cc: ross.philip...@oracle.com
Cc: Thomas Gleixner 
Cc: x86-ml 
Cc: xen-devel@lists.xenproject.org
Link: https://lkml.kernel.org/r/20191112134640.16035-4-daniel.ki...@oracle.com
---
 Documentation/x86/boot.rst | 43 -
 arch/x86/boot/compressed/kaslr.c   | 12 +++-
 arch/x86/boot/compressed/kernel_info.S |  2 +-
 arch/x86/boot/header.S |  2 +-
 arch/x86/include/uapi/asm/bootparam.h  | 16 +++--
 arch/x86/kernel/e820.c | 11 ++-
 arch/x86/kernel/kdebugfs.c | 21 +---
 arch/x86/kernel/ksysfs.c   | 31 +-
 arch/x86/kernel/setup.c|  6 +++-
 arch/x86/mm/ioremap.c  | 11 ++-
 10 files changed, 138 insertions(+), 17 deletions(-)

diff --git a/Documentation/x86/boot.rst b/Documentation/x86/boot.rst
index 6cdd767..90bb8f5 100644
--- a/Documentation/x86/boot.rst
+++ b/Documentation/x86/boot.rst
@@ -827,6 +827,47 @@ Protocol:  2.09+
   sure to consider the case where the linked list already contains
   entries.
 
+  The setup_data is a bit awkward to use for extremely large data objects,
+  both because the setup_data header has to be adjacent to the data object
+  and because it has a 32-bit length field. However, it is important that
+  intermediate stages of the boot process have a way to identify which
+  chunks of memory are occupied by kernel data.
+
+  Thus setup_indirect struct and SETUP_INDIRECT type were introduced in
+  protocol 2.15.
+
+  struct setup_indirect {
+__u32 type;
+__u32 reserved;  /* Reserved, must be set to zero. */
+__u64 len;
+__u64 addr;
+  };
+
+  The type member is a SETUP_INDIRECT | SETUP_* type. However, it cannot be
+  SETUP_INDIRECT itself since making the setup_indirect a tree structure
+  could require a lot of stack space in something that needs to parse it
+  and stack space can be limited in boot contexts.
+
+  Let's give an example how to point to SETUP_E820_EXT data using 
setup_indirect.
+  In this case setup_data and setup_indirect will look like this:
+
+  struct setup_data {
+__u64 next = 0 or ;
+__u32 type = SETUP_INDIRECT;
+__u32 len = sizeof(setup_data);
+__u8 data[sizeof(setup_indirect)] = struct setup_indirect {
+  __u32 type = SETUP_INDIRECT | SETUP_E820_EXT;
+  __u32 reserved = 0;
+  __u64 len = ;
+  __u64 addr = ;
+}
+  }
+
+.. note::
+ SETUP_INDIRECT | SETUP_NONE objects cannot be properly distinguished
+ from SETUP_INDIRECT itself. So, this kind of objects cannot be provided
+ by the bootloaders.
+
    
 Field name:pref_address
 Type:  read (reloc)
@@ -986,7 +1027,7 @@ Field name:setup_type_max
 Offset/size:   0x000c/4
    ==
 
-  This field contains maximal allowed type for setup_data.
+  This field contains maximal allowed type for setup_data and setup_indirect 
structs.
 
 
 The Image Checksum
diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index 2e53c05..bb9bfef 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -459,6 +459,18 @@ static bool mem_avoid_overlap(struct mem_vector *img,
is_overlapping = true;
}
 
+   if (ptr->type == SETUP_INDIRECT &&
+   ((struct setup_indirect *)ptr->data)->type != 
SETUP_INDIRECT) {
+   avoid.start = ((struct setup_indirect 
*)ptr->data)->addr;
+ 

[Xen-devel] [PATCH v6 1/3] x86/boot: Introduce the kernel_info

[Daniel Kiper]

The relationships between the headers are analogous to the various data
sections:

  setup_header = .data
  boot_params/setup_data = .bss

What is missing from the above list? That's right:

  kernel_info = .rodata

We have been (ab)using .data for things that could go into .rodata or .bss for
a long time, for lack of alternatives and -- especially early on -- inertia.
Also, the BIOS stub is responsible for creating boot_params, so it isn't
available to a BIOS-based loader (setup_data is, though).

setup_header is permanently limited to 144 bytes due to the reach of the
2-byte jump field, which doubles as a length field for the structure, combined
with the size of the "hole" in struct boot_params that a protected-mode loader
or the BIOS stub has to copy it into. It is currently 119 bytes long, which
leaves us with 25 very precious bytes. This isn't something that can be fixed
without revising the boot protocol entirely, breaking backwards compatibility.

boot_params proper is limited to 4096 bytes, but can be arbitrarily extended
by adding setup_data entries. It cannot be used to communicate properties of
the kernel image, because it is .bss and has no image-provided content.

kernel_info solves this by providing an extensible place for information about
the kernel image. It is readonly, because the kernel cannot rely on a
bootloader copying its contents anywhere, but that is OK; if it becomes
necessary it can still contain data items that an enabled bootloader would be
expected to copy into a setup_data chunk.

Do not bump setup_header version in arch/x86/boot/header.S because it
will be followed by additional changes coming into the Linux/x86 boot
protocol.

Suggested-by: H. Peter Anvin (Intel) 
Signed-off-by: Daniel Kiper 
Reviewed-by: Konrad Rzeszutek Wilk 
Reviewed-by: Ross Philipson 
Reviewed-by: H. Peter Anvin (Intel) 
---
v6 - suggestions/fixes:
   - drop "This patch" from the commit message
 (suggested by Borislav Petkov).

v4 - suggestions/fixes:
   - improve the documentation
 (suggested by Randy Dunlap and Konrad Rzeszutek Wilk).

v3 - suggestions/fixes:
   - split kernel_info data into fixed and variable sized regions,
 (suggested by H. Peter Anvin),
   - change kernel_info.header value to "LToP" (0x506f544c),
 (suggested by H. Peter Anvin),
   - improve the comments,
   - improve the documentation.

v2 - suggestions/fixes:
   - rename setup_header2 to kernel_info,
 (suggested by H. Peter Anvin),
   - change kernel_info.header value to "InfO" (0x4f666e49),
   - new kernel_info description in Documentation/x86/boot.rst,
 (suggested by H. Peter Anvin),
   - drop kernel_info_offset_update() as an overkill and
 update kernel_info offset directly from main(),
 (suggested by Eric Snowberg),
   - new commit message
 (suggested by H. Peter Anvin),
   - fix some commit message misspellings
 (suggested by Eric Snowberg).
---
 Documentation/x86/boot.rst | 126 +
 arch/x86/boot/Makefile |   2 +-
 arch/x86/boot/compressed/Makefile  |   4 +-
 arch/x86/boot/compressed/kernel_info.S |  17 +
 arch/x86/boot/header.S |   1 +
 arch/x86/boot/tools/build.c|   5 ++
 arch/x86/include/uapi/asm/bootparam.h  |   1 +
 7 files changed, 153 insertions(+), 3 deletions(-)
 create mode 100644 arch/x86/boot/compressed/kernel_info.S

diff --git a/Documentation/x86/boot.rst b/Documentation/x86/boot.rst
index 08a2f100c0e6..c60fafda9427 100644
--- a/Documentation/x86/boot.rst
+++ b/Documentation/x86/boot.rst
@@ -68,8 +68,25 @@ Protocol 2.12(Kernel 3.8) Added the xloadflags field 
and extension fields
 Protocol 2.13  (Kernel 3.14) Support 32- and 64-bit flags being set in
xloadflags to support booting a 64-bit kernel from 32-bit
EFI
+
+Protocol 2.14: BURNT BY INCORRECT COMMIT 
ae7e1238e68f2a472a125673ab506d49158c1889
+   (x86/boot: Add ACPI RSDP address to setup_header)
+   DO NOT USE!!! ASSUME SAME AS 2.13.
+
+Protocol 2.15: (Kernel 5.5) Added the kernel_info.
 =  
 
+.. note::
+ The protocol version number should be changed only if the setup header
+ is changed. There is no need to update the version number if boot_params
+ or kernel_info are changed. Additionally, it is recommended to use
+ xloadflags (in this case the protocol version number should not be
+ updated either) or kernel_info to communicate supported Linux kernel
+ features to the boot loader. Due to very limited space available in
+ the original setup header every update to it should be considered
+ with great care. Starting from the protocol 2.15 the primary way to
+ communicate things to the boot loader is the kernel_info.
+
 
 Memory Layout
 =
@@ -207,6 +224,7 @@ Offset/Size Proto   Name

[Xen-devel] [PATCH v6 2/3] x86/boot: Introduce the kernel_info.setup_type_max

[Daniel Kiper]

This field contains maximal allowed type for setup_data.

Do not bump setup_header version in arch/x86/boot/header.S because it
will be followed by additional changes coming into the Linux/x86 boot
protocol.

Suggested-by: H. Peter Anvin (Intel) 
Signed-off-by: Daniel Kiper 
Reviewed-by: Konrad Rzeszutek Wilk 
Reviewed-by: Ross Philipson 
Reviewed-by: H. Peter Anvin (Intel) 
---
v6 - suggestions/fixes:
   - fix setup_type_max offset in Documentation/x86/boot.rst
 (suggested by Borislav Petkov),
   - drop "This patch" from the commit message
 (suggested by Borislav Petkov).

v5 - suggestions/fixes:
   - move incorrect references to the setup_indirect to the
 patch introducing it,
   - do not bump setup_header version in arch/x86/boot/header.S
 (suggested by H. Peter Anvin).
---
 Documentation/x86/boot.rst | 9 -
 arch/x86/boot/compressed/kernel_info.S | 5 +
 arch/x86/include/uapi/asm/bootparam.h  | 3 +++
 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/Documentation/x86/boot.rst b/Documentation/x86/boot.rst
index c60fafda9427..6cdd767c3835 100644
--- a/Documentation/x86/boot.rst
+++ b/Documentation/x86/boot.rst
@@ -73,7 +73,7 @@ Protocol 2.14:BURNT BY INCORRECT COMMIT 
ae7e1238e68f2a472a125673ab506d49158c188
(x86/boot: Add ACPI RSDP address to setup_header)
DO NOT USE!!! ASSUME SAME AS 2.13.
 
-Protocol 2.15: (Kernel 5.5) Added the kernel_info.
+Protocol 2.15: (Kernel 5.5) Added the kernel_info and 
kernel_info.setup_type_max.
 =  
 
 .. note::
@@ -981,6 +981,13 @@ Offset/size:   0x0008/4
   This field contains the size of the kernel_info including kernel_info.header
   and kernel_info.kernel_info_var_len_data.
 
+   ==
+Field name:setup_type_max
+Offset/size:   0x000c/4
+   ==
+
+  This field contains maximal allowed type for setup_data.
+
 
 The Image Checksum
 ==
diff --git a/arch/x86/boot/compressed/kernel_info.S 
b/arch/x86/boot/compressed/kernel_info.S
index 8ea6f6e3feef..018dacbd753e 100644
--- a/arch/x86/boot/compressed/kernel_info.S
+++ b/arch/x86/boot/compressed/kernel_info.S
@@ -1,5 +1,7 @@
 /* SPDX-License-Identifier: GPL-2.0 */
 
+#include 
+
.section ".rodata.kernel_info", "a"
 
.global kernel_info
@@ -12,6 +14,9 @@ kernel_info:
/* Size total. */
.long   kernel_info_end - kernel_info
 
+   /* Maximal allowed type for setup_data. */
+   .long   SETUP_TYPE_MAX
+
 kernel_info_var_len_data:
/* Empty for time being... */
 kernel_info_end:
diff --git a/arch/x86/include/uapi/asm/bootparam.h 
b/arch/x86/include/uapi/asm/bootparam.h
index a1ebcd7a991c..dbb41128e5a0 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -11,6 +11,9 @@
 #define SETUP_APPLE_PROPERTIES 5
 #define SETUP_JAILHOUSE6
 
+/* max(SETUP_*) */
+#define SETUP_TYPE_MAX SETUP_JAILHOUSE
+
 /* ram_size flags */
 #define RAMDISK_IMAGE_START_MASK   0x07FF
 #define RAMDISK_PROMPT_FLAG0x8000
-- 
2.11.0


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

[Xen-devel] [PATCH v6 0/3] x86/boot: Introduce the kernel_info et consortes

[Daniel Kiper]

Hi,

Due to very limited space in the setup_header this patch series introduces new
kernel_info struct which will be used to convey information from the kernel to
the bootloader. This way the boot protocol can be extended regardless of the
setup_header limitations. Additionally, the patch series introduces some
convenience features like the setup_indirect struct and the
kernel_info.setup_type_max field.

Daniel

 Documentation/x86/boot.rst | 174 
++
 arch/x86/boot/Makefile |   2 +-
 arch/x86/boot/compressed/Makefile  |   4 +-
 arch/x86/boot/compressed/kaslr.c   |  12 ++
 arch/x86/boot/compressed/kernel_info.S |  22 ++
 arch/x86/boot/header.S |   3 +-
 arch/x86/boot/tools/build.c|   5 +++
 arch/x86/include/uapi/asm/bootparam.h  |  16 +++-
 arch/x86/kernel/e820.c |  11 +
 arch/x86/kernel/kdebugfs.c |  21 --
 arch/x86/kernel/ksysfs.c   |  31 ++
 arch/x86/kernel/setup.c|   6 +++
 arch/x86/mm/ioremap.c  |  11 +
 13 files changed, 302 insertions(+), 16 deletions(-)

Daniel Kiper (3):
  x86/boot: Introduce the kernel_info
  x86/boot: Introduce the kernel_info.setup_type_max
  x86/boot: Introduce the setup_indirect


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

[Xen-devel] [PATCH v6 3/3] x86/boot: Introduce the setup_indirect

[Daniel Kiper]

The setup_data is a bit awkward to use for extremely large data objects,
both because the setup_data header has to be adjacent to the data object
and because it has a 32-bit length field. However, it is important that
intermediate stages of the boot process have a way to identify which
chunks of memory are occupied by kernel data. Thus introduce an uniform
way to specify such indirect data as setup_indirect struct and
SETUP_INDIRECT type.

And finally bump setup_header version in arch/x86/boot/header.S.

Suggested-by: H. Peter Anvin (Intel) 
Signed-off-by: Daniel Kiper 
Acked-by: Konrad Rzeszutek Wilk 
Reviewed-by: Ross Philipson 
Reviewed-by: H. Peter Anvin (Intel) 
---
v6 - suggestions/fixes:
   - add a comment to arch/x86/kernel/kdebugfs.c
 (suggested by Borislav Petkov),
   - do some formatting tricks to increase code readability
 (suggested by Borislav Petkov),
   - drop "we" from the commit message
 (suggested by Borislav Petkov).

v5 - suggestions/fixes:
   - bump setup_header version in arch/x86/boot/header.S
 (suggested by H. Peter Anvin).

v4 - suggestions/fixes:
   - change "Note:" to ".. note::".

v3 - suggestions/fixes:
   - add setup_indirect mapping/KASLR avoidance/etc. code
 (suggested by H. Peter Anvin),
   - the SETUP_INDIRECT sets most significant bit right now;
 this way it is possible to differentiate regular setup_data
 and setup_indirect objects in the debugfs filesystem.

v2 - suggestions/fixes:
   - add setup_indirect usage example
 (suggested by Eric Snowberg and Ross Philipson).
---
 Documentation/x86/boot.rst | 43 +-
 arch/x86/boot/compressed/kaslr.c   | 12 ++
 arch/x86/boot/compressed/kernel_info.S |  2 +-
 arch/x86/boot/header.S |  2 +-
 arch/x86/include/uapi/asm/bootparam.h  | 16 ++---
 arch/x86/kernel/e820.c | 11 +
 arch/x86/kernel/kdebugfs.c | 21 +
 arch/x86/kernel/ksysfs.c   | 31 ++--
 arch/x86/kernel/setup.c|  6 +
 arch/x86/mm/ioremap.c  | 11 +
 10 files changed, 138 insertions(+), 17 deletions(-)

diff --git a/Documentation/x86/boot.rst b/Documentation/x86/boot.rst
index 6cdd767c3835..90bb8f5ab384 100644
--- a/Documentation/x86/boot.rst
+++ b/Documentation/x86/boot.rst
@@ -827,6 +827,47 @@ Protocol:  2.09+
   sure to consider the case where the linked list already contains
   entries.
 
+  The setup_data is a bit awkward to use for extremely large data objects,
+  both because the setup_data header has to be adjacent to the data object
+  and because it has a 32-bit length field. However, it is important that
+  intermediate stages of the boot process have a way to identify which
+  chunks of memory are occupied by kernel data.
+
+  Thus setup_indirect struct and SETUP_INDIRECT type were introduced in
+  protocol 2.15.
+
+  struct setup_indirect {
+__u32 type;
+__u32 reserved;  /* Reserved, must be set to zero. */
+__u64 len;
+__u64 addr;
+  };
+
+  The type member is a SETUP_INDIRECT | SETUP_* type. However, it cannot be
+  SETUP_INDIRECT itself since making the setup_indirect a tree structure
+  could require a lot of stack space in something that needs to parse it
+  and stack space can be limited in boot contexts.
+
+  Let's give an example how to point to SETUP_E820_EXT data using 
setup_indirect.
+  In this case setup_data and setup_indirect will look like this:
+
+  struct setup_data {
+__u64 next = 0 or ;
+__u32 type = SETUP_INDIRECT;
+__u32 len = sizeof(setup_data);
+__u8 data[sizeof(setup_indirect)] = struct setup_indirect {
+  __u32 type = SETUP_INDIRECT | SETUP_E820_EXT;
+  __u32 reserved = 0;
+  __u64 len = ;
+  __u64 addr = ;
+}
+  }
+
+.. note::
+ SETUP_INDIRECT | SETUP_NONE objects cannot be properly distinguished
+ from SETUP_INDIRECT itself. So, this kind of objects cannot be provided
+ by the bootloaders.
+
    
 Field name:pref_address
 Type:  read (reloc)
@@ -986,7 +1027,7 @@ Field name:setup_type_max
 Offset/size:   0x000c/4
    ==
 
-  This field contains maximal allowed type for setup_data.
+  This field contains maximal allowed type for setup_data and setup_indirect 
structs.
 
 
 The Image Checksum
diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index 2e53c056ba20..bb9bfef174ae 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -459,6 +459,18 @@ static bool mem_avoid_overlap(struct mem_vector *img,
is_overlapping = true;
}
 
+   if (ptr->type == SETUP_INDIRECT &&
+   ((struct setup_indirect *)ptr->data)->type != 
SETUP_INDIRECT) {
+   avoid.star

Re: [Xen-devel] [PATCH v5 2/3] x86/boot: Introduce the kernel_info.setup_type_max

[Daniel Kiper]

On Fri, Nov 08, 2019 at 02:03:38PM +0100, Borislav Petkov wrote:
> On Fri, Nov 08, 2019 at 01:52:48PM +0100, Daniel Kiper wrote:
> > OK, got your comments. I will repost the patch series probably on Tuesday.
> > I hope that it will land in 5.5 then.
>
> I don't see why not if you base it ontop of tip:x86/boot and test it
> properly before sending.

Great!

> Out of curiosity, is there any particular reason this should be in 5.5?

Just want to have it done... :-))) ...and continue work on stuff which
depends on it.

Daniel

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

Re: [Xen-devel] [PATCH v5 2/3] x86/boot: Introduce the kernel_info.setup_type_max

[Daniel Kiper]

On Fri, Nov 08, 2019 at 12:07:03PM +0100, Borislav Petkov wrote:
> On Fri, Nov 08, 2019 at 11:47:02AM +0100, Daniel Kiper wrote:
> > Yeah, you are right. Would you like me to repost whole patch series or
> > could you fix it before committing?
>
> Lemme finish looking at patch 3 first.
>
> If you have to resend, please remove "This patch" and "We" in your text.

OK, got your comments. I will repost the patch series probably on Tuesday.
I hope that it will land in 5.5 then.

Daniel

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

Re: [Xen-devel] [PATCH v5 2/3] x86/boot: Introduce the kernel_info.setup_type_max

[Daniel Kiper]

On Fri, Nov 08, 2019 at 11:09:30AM +0100, Borislav Petkov wrote:
> On Mon, Nov 04, 2019 at 04:13:53PM +0100, Daniel Kiper wrote:
> > This field contains maximal allowed type for setup_data.
> >
> > This patch does not bump setup_header version in arch/x86/boot/header.S
> > because it will be followed by additional changes coming into the
> > Linux/x86 boot protocol.
> >
> > Suggested-by: H. Peter Anvin (Intel) 
> > Signed-off-by: Daniel Kiper 
> > Reviewed-by: Konrad Rzeszutek Wilk 
> > Reviewed-by: Ross Philipson 
> > Reviewed-by: H. Peter Anvin (Intel) 
> > ---
> > v5 - suggestions/fixes:
> >- move incorrect references to the setup_indirect to the
> >  patch introducing it,
> >- do not bump setup_header version in arch/x86/boot/header.S
> >  (suggested by H. Peter Anvin).
> > ---
> >  Documentation/x86/boot.rst | 9 -
> >  arch/x86/boot/compressed/kernel_info.S | 5 +
> >  arch/x86/include/uapi/asm/bootparam.h  | 3 +++
> >  3 files changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/x86/boot.rst b/Documentation/x86/boot.rst
> > index c60fafda9427..1dad6eee8a5c 100644
> > --- a/Documentation/x86/boot.rst
> > +++ b/Documentation/x86/boot.rst
> > @@ -73,7 +73,7 @@ Protocol 2.14:BURNT BY INCORRECT COMMIT 
> > ae7e1238e68f2a472a125673ab506d49158c188
> > (x86/boot: Add ACPI RSDP address to setup_header)
> > DO NOT USE!!! ASSUME SAME AS 2.13.
> >
> > -Protocol 2.15: (Kernel 5.5) Added the kernel_info.
> > +Protocol 2.15: (Kernel 5.5) Added the kernel_info and 
> > kernel_info.setup_type_max.
> >  =  
> > 
> >
> >  .. note::
> > @@ -981,6 +981,13 @@ Offset/size:   0x0008/4
> >This field contains the size of the kernel_info including 
> > kernel_info.header
> >and kernel_info.kernel_info_var_len_data.
> >
> > +   ==
> > +Field name:setup_type_max
> > +Offset/size:   0x0008/4
>
> You already have
>
> Field name: size_total
> Offset/size:0x0008/4
>
> at that offset.
>
> I guess you mean setup_type_max's offset to be 0x000c and it would be
> that member:
>
> .long   0x01234567  /* Some fixed size data for the bootloaders. */
>
> ?

Yeah, you are right. Would you like me to repost whole patch series or
could you fix it before committing?

Daniel

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

Re: [Xen-devel] [PATCH v5 0/3] x86/boot: Introduce the kernel_info et consortes

[Daniel Kiper]

On Wed, Nov 06, 2019 at 09:56:48AM -0800, h...@zytor.com wrote:
> On November 6, 2019 9:03:33 AM PST, Borislav Petkov  wrote:
> >On Mon, Nov 04, 2019 at 04:13:51PM +0100, Daniel Kiper wrote:
> >> Hi,
> >>
> >> Due to very limited space in the setup_header this patch series introduces 
> >> new
> >> kernel_info struct which will be used to convey information from the 
> >> kernel to
> >> the bootloader. This way the boot protocol can be extended regardless of 
> >> the
> >> setup_header limitations. Additionally, the patch series introduces some
> >> convenience features like the setup_indirect struct and the
> >> kernel_info.setup_type_max field.
> >
> >That's all fine and dandy but I'm missing an example about what that'll
> >be used for, in practice.
> >
> >Thx.
>
> For one thing, we already have people asking for more than 4 GiB worth
> of initramfs, and especially with initramfs that huge it would make a
> *lot* of sense to allow loading it in chunks without having to
> concatenate them. I have been asking for a long time for initramfs
> creators to split the kernel-dependent and kernel independent parts
> into separate initramfs modules.

Another user of this patchset is the TrenchBoot project on which we are
working on. We have to introduce separate entry point for Intel TXT MLE
startup code. That is why we need the kernel_info struct.

Daniel

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

Re: [Xen-devel] [PATCH v4 2/3] x86/boot: Introduce the kernel_info.setup_type_max

[Daniel Kiper]

On Fri, Nov 01, 2019 at 01:55:57PM -0700, H. Peter Anvin wrote:
> On 2019-10-24 04:48, Daniel Kiper wrote:
> > This field contains maximal allowed type for setup_data.
> >
> > Now bump the setup_header version in arch/x86/boot/header.S.
>
> Please don't bump the protocol revision here, otherwise we would create
> a very odd pseudo-revision of the protocol: 2.15 without SETUP_INDIRECT
> support, should patch 3/3 end up getting reverted.
>
> (It is possible to detect, of course, but I feel pretty sure in saying
> that bootloaders won't get it right.)
>
> Other than that:
>
> Reviewed-by: H. Peter Anvin (Intel) 

I have just sent v5. Please take a look.

Daniel

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

[Xen-devel] [PATCH v5 0/3] x86/boot: Introduce the kernel_info et consortes

[Daniel Kiper]

Hi,

Due to very limited space in the setup_header this patch series introduces new
kernel_info struct which will be used to convey information from the kernel to
the bootloader. This way the boot protocol can be extended regardless of the
setup_header limitations. Additionally, the patch series introduces some
convenience features like the setup_indirect struct and the
kernel_info.setup_type_max field.

Daniel

 Documentation/x86/boot.rst | 174 
++
 arch/x86/boot/Makefile |   2 +-
 arch/x86/boot/compressed/Makefile  |   4 +-
 arch/x86/boot/compressed/kaslr.c   |  12 ++
 arch/x86/boot/compressed/kernel_info.S |  22 ++
 arch/x86/boot/header.S |   3 +-
 arch/x86/boot/tools/build.c|   5 +++
 arch/x86/include/uapi/asm/bootparam.h  |  16 +++-
 arch/x86/kernel/e820.c |  11 +
 arch/x86/kernel/kdebugfs.c |  20 +++--
 arch/x86/kernel/ksysfs.c   |  30 ++
 arch/x86/kernel/setup.c|   4 ++
 arch/x86/mm/ioremap.c  |  11 +
 13 files changed, 298 insertions(+), 16 deletions(-)

Daniel Kiper (3):
  x86/boot: Introduce the kernel_info
  x86/boot: Introduce the kernel_info.setup_type_max
  x86/boot: Introduce the setup_indirect


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

[Xen-devel] [PATCH v5 2/3] x86/boot: Introduce the kernel_info.setup_type_max

[Daniel Kiper]

This field contains maximal allowed type for setup_data.

This patch does not bump setup_header version in arch/x86/boot/header.S
because it will be followed by additional changes coming into the
Linux/x86 boot protocol.

Suggested-by: H. Peter Anvin (Intel) 
Signed-off-by: Daniel Kiper 
Reviewed-by: Konrad Rzeszutek Wilk 
Reviewed-by: Ross Philipson 
Reviewed-by: H. Peter Anvin (Intel) 
---
v5 - suggestions/fixes:
   - move incorrect references to the setup_indirect to the
 patch introducing it,
   - do not bump setup_header version in arch/x86/boot/header.S
 (suggested by H. Peter Anvin).
---
 Documentation/x86/boot.rst | 9 -
 arch/x86/boot/compressed/kernel_info.S | 5 +
 arch/x86/include/uapi/asm/bootparam.h  | 3 +++
 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/Documentation/x86/boot.rst b/Documentation/x86/boot.rst
index c60fafda9427..1dad6eee8a5c 100644
--- a/Documentation/x86/boot.rst
+++ b/Documentation/x86/boot.rst
@@ -73,7 +73,7 @@ Protocol 2.14:BURNT BY INCORRECT COMMIT 
ae7e1238e68f2a472a125673ab506d49158c188
(x86/boot: Add ACPI RSDP address to setup_header)
DO NOT USE!!! ASSUME SAME AS 2.13.
 
-Protocol 2.15: (Kernel 5.5) Added the kernel_info.
+Protocol 2.15: (Kernel 5.5) Added the kernel_info and 
kernel_info.setup_type_max.
 =  
 
 .. note::
@@ -981,6 +981,13 @@ Offset/size:   0x0008/4
   This field contains the size of the kernel_info including kernel_info.header
   and kernel_info.kernel_info_var_len_data.
 
+   ==
+Field name:setup_type_max
+Offset/size:   0x0008/4
+   ==
+
+  This field contains maximal allowed type for setup_data.
+
 
 The Image Checksum
 ==
diff --git a/arch/x86/boot/compressed/kernel_info.S 
b/arch/x86/boot/compressed/kernel_info.S
index 8ea6f6e3feef..018dacbd753e 100644
--- a/arch/x86/boot/compressed/kernel_info.S
+++ b/arch/x86/boot/compressed/kernel_info.S
@@ -1,5 +1,7 @@
 /* SPDX-License-Identifier: GPL-2.0 */
 
+#include 
+
.section ".rodata.kernel_info", "a"
 
.global kernel_info
@@ -12,6 +14,9 @@ kernel_info:
/* Size total. */
.long   kernel_info_end - kernel_info
 
+   /* Maximal allowed type for setup_data. */
+   .long   SETUP_TYPE_MAX
+
 kernel_info_var_len_data:
/* Empty for time being... */
 kernel_info_end:
diff --git a/arch/x86/include/uapi/asm/bootparam.h 
b/arch/x86/include/uapi/asm/bootparam.h
index a1ebcd7a991c..dbb41128e5a0 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -11,6 +11,9 @@
 #define SETUP_APPLE_PROPERTIES 5
 #define SETUP_JAILHOUSE6
 
+/* max(SETUP_*) */
+#define SETUP_TYPE_MAX SETUP_JAILHOUSE
+
 /* ram_size flags */
 #define RAMDISK_IMAGE_START_MASK   0x07FF
 #define RAMDISK_PROMPT_FLAG0x8000
-- 
2.11.0


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

[Xen-devel] [PATCH v5 1/3] x86/boot: Introduce the kernel_info

[Daniel Kiper]

The relationships between the headers are analogous to the various data
sections:

  setup_header = .data
  boot_params/setup_data = .bss

What is missing from the above list? That's right:

  kernel_info = .rodata

We have been (ab)using .data for things that could go into .rodata or .bss for
a long time, for lack of alternatives and -- especially early on -- inertia.
Also, the BIOS stub is responsible for creating boot_params, so it isn't
available to a BIOS-based loader (setup_data is, though).

setup_header is permanently limited to 144 bytes due to the reach of the
2-byte jump field, which doubles as a length field for the structure, combined
with the size of the "hole" in struct boot_params that a protected-mode loader
or the BIOS stub has to copy it into. It is currently 119 bytes long, which
leaves us with 25 very precious bytes. This isn't something that can be fixed
without revising the boot protocol entirely, breaking backwards compatibility.

boot_params proper is limited to 4096 bytes, but can be arbitrarily extended
by adding setup_data entries. It cannot be used to communicate properties of
the kernel image, because it is .bss and has no image-provided content.

kernel_info solves this by providing an extensible place for information about
the kernel image. It is readonly, because the kernel cannot rely on a
bootloader copying its contents anywhere, but that is OK; if it becomes
necessary it can still contain data items that an enabled bootloader would be
expected to copy into a setup_data chunk.

This patch does not bump setup_header version in arch/x86/boot/header.S
because it will be followed by additional changes coming into the
Linux/x86 boot protocol.

Suggested-by: H. Peter Anvin (Intel) 
Signed-off-by: Daniel Kiper 
Reviewed-by: Konrad Rzeszutek Wilk 
Reviewed-by: Ross Philipson 
Reviewed-by: H. Peter Anvin (Intel) 
---
v4 - suggestions/fixes:
   - improve the documentation
 (suggested by Randy Dunlap and Konrad Rzeszutek Wilk).

v3 - suggestions/fixes:
   - split kernel_info data into fixed and variable sized regions,
 (suggested by H. Peter Anvin),
   - change kernel_info.header value to "LToP" (0x506f544c),
 (suggested by H. Peter Anvin),
   - improve the comments,
   - improve the documentation.

v2 - suggestions/fixes:
   - rename setup_header2 to kernel_info,
 (suggested by H. Peter Anvin),
   - change kernel_info.header value to "InfO" (0x4f666e49),
   - new kernel_info description in Documentation/x86/boot.rst,
 (suggested by H. Peter Anvin),
   - drop kernel_info_offset_update() as an overkill and
 update kernel_info offset directly from main(),
 (suggested by Eric Snowberg),
   - new commit message
 (suggested by H. Peter Anvin),
   - fix some commit message misspellings
 (suggested by Eric Snowberg).
---
 Documentation/x86/boot.rst | 126 +
 arch/x86/boot/Makefile |   2 +-
 arch/x86/boot/compressed/Makefile  |   4 +-
 arch/x86/boot/compressed/kernel_info.S |  17 +
 arch/x86/boot/header.S |   1 +
 arch/x86/boot/tools/build.c|   5 ++
 arch/x86/include/uapi/asm/bootparam.h  |   1 +
 7 files changed, 153 insertions(+), 3 deletions(-)
 create mode 100644 arch/x86/boot/compressed/kernel_info.S

diff --git a/Documentation/x86/boot.rst b/Documentation/x86/boot.rst
index 08a2f100c0e6..c60fafda9427 100644
--- a/Documentation/x86/boot.rst
+++ b/Documentation/x86/boot.rst
@@ -68,8 +68,25 @@ Protocol 2.12(Kernel 3.8) Added the xloadflags field 
and extension fields
 Protocol 2.13  (Kernel 3.14) Support 32- and 64-bit flags being set in
xloadflags to support booting a 64-bit kernel from 32-bit
EFI
+
+Protocol 2.14: BURNT BY INCORRECT COMMIT 
ae7e1238e68f2a472a125673ab506d49158c1889
+   (x86/boot: Add ACPI RSDP address to setup_header)
+   DO NOT USE!!! ASSUME SAME AS 2.13.
+
+Protocol 2.15: (Kernel 5.5) Added the kernel_info.
 =  
 
+.. note::
+ The protocol version number should be changed only if the setup header
+ is changed. There is no need to update the version number if boot_params
+ or kernel_info are changed. Additionally, it is recommended to use
+ xloadflags (in this case the protocol version number should not be
+ updated either) or kernel_info to communicate supported Linux kernel
+ features to the boot loader. Due to very limited space available in
+ the original setup header every update to it should be considered
+ with great care. Starting from the protocol 2.15 the primary way to
+ communicate things to the boot loader is the kernel_info.
+
 
 Memory Layout
 =
@@ -207,6 +224,7 @@ Offset/Size Proto   NameMeaning
 0258/8 2.10+   pref_addressPreferred loading

[Xen-devel] [PATCH v5 3/3] x86/boot: Introduce the setup_indirect

[Daniel Kiper]

The setup_data is a bit awkward to use for extremely large data objects,
both because the setup_data header has to be adjacent to the data object
and because it has a 32-bit length field. However, it is important that
intermediate stages of the boot process have a way to identify which
chunks of memory are occupied by kernel data. Thus we introduce an uniform
way to specify such indirect data as setup_indirect struct and
SETUP_INDIRECT type.

And finally bump setup_header version in arch/x86/boot/header.S.

Suggested-by: H. Peter Anvin (Intel) 
Signed-off-by: Daniel Kiper 
Acked-by: Konrad Rzeszutek Wilk 
Reviewed-by: Ross Philipson 
Reviewed-by: H. Peter Anvin (Intel) 
---
v5 - suggestions/fixes:
   - bump setup_header version in arch/x86/boot/header.S
 (suggested by H. Peter Anvin).

v4 - suggestions/fixes:
   - change "Note:" to ".. note::".

v3 - suggestions/fixes:
   - add setup_indirect mapping/KASLR avoidance/etc. code
 (suggested by H. Peter Anvin),
   - the SETUP_INDIRECT sets most significant bit right now;
 this way it is possible to differentiate regular setup_data
 and setup_indirect objects in the debugfs filesystem.

v2 - suggestions/fixes:
   - add setup_indirect usage example
 (suggested by Eric Snowberg and Ross Philipson).
---
 Documentation/x86/boot.rst | 43 +-
 arch/x86/boot/compressed/kaslr.c   | 12 ++
 arch/x86/boot/compressed/kernel_info.S |  2 +-
 arch/x86/boot/header.S |  2 +-
 arch/x86/include/uapi/asm/bootparam.h  | 16 ++---
 arch/x86/kernel/e820.c | 11 +
 arch/x86/kernel/kdebugfs.c | 20 
 arch/x86/kernel/ksysfs.c   | 30 ++--
 arch/x86/kernel/setup.c|  4 
 arch/x86/mm/ioremap.c  | 11 +
 10 files changed, 134 insertions(+), 17 deletions(-)

diff --git a/Documentation/x86/boot.rst b/Documentation/x86/boot.rst
index 1dad6eee8a5c..38155ba8740f 100644
--- a/Documentation/x86/boot.rst
+++ b/Documentation/x86/boot.rst
@@ -827,6 +827,47 @@ Protocol:  2.09+
   sure to consider the case where the linked list already contains
   entries.
 
+  The setup_data is a bit awkward to use for extremely large data objects,
+  both because the setup_data header has to be adjacent to the data object
+  and because it has a 32-bit length field. However, it is important that
+  intermediate stages of the boot process have a way to identify which
+  chunks of memory are occupied by kernel data.
+
+  Thus setup_indirect struct and SETUP_INDIRECT type were introduced in
+  protocol 2.15.
+
+  struct setup_indirect {
+__u32 type;
+__u32 reserved;  /* Reserved, must be set to zero. */
+__u64 len;
+__u64 addr;
+  };
+
+  The type member is a SETUP_INDIRECT | SETUP_* type. However, it cannot be
+  SETUP_INDIRECT itself since making the setup_indirect a tree structure
+  could require a lot of stack space in something that needs to parse it
+  and stack space can be limited in boot contexts.
+
+  Let's give an example how to point to SETUP_E820_EXT data using 
setup_indirect.
+  In this case setup_data and setup_indirect will look like this:
+
+  struct setup_data {
+__u64 next = 0 or ;
+__u32 type = SETUP_INDIRECT;
+__u32 len = sizeof(setup_data);
+__u8 data[sizeof(setup_indirect)] = struct setup_indirect {
+  __u32 type = SETUP_INDIRECT | SETUP_E820_EXT;
+  __u32 reserved = 0;
+  __u64 len = ;
+  __u64 addr = ;
+}
+  }
+
+.. note::
+ SETUP_INDIRECT | SETUP_NONE objects cannot be properly distinguished
+ from SETUP_INDIRECT itself. So, this kind of objects cannot be provided
+ by the bootloaders.
+
    
 Field name:pref_address
 Type:  read (reloc)
@@ -986,7 +1027,7 @@ Field name:setup_type_max
 Offset/size:   0x0008/4
    ==
 
-  This field contains maximal allowed type for setup_data.
+  This field contains maximal allowed type for setup_data and setup_indirect 
structs.
 
 
 The Image Checksum
diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index 2e53c056ba20..bb9bfef174ae 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -459,6 +459,18 @@ static bool mem_avoid_overlap(struct mem_vector *img,
is_overlapping = true;
}
 
+   if (ptr->type == SETUP_INDIRECT &&
+   ((struct setup_indirect *)ptr->data)->type != 
SETUP_INDIRECT) {
+   avoid.start = ((struct setup_indirect 
*)ptr->data)->addr;
+   avoid.size = ((struct setup_indirect *)ptr->data)->len;
+
+   if (mem_overlaps(img, ) && (avoid.start < 
earliest)) {
+   *overlap = avoi

[Xen-devel] [PATCH v4 3/3] x86/boot: Introduce the setup_indirect

[Daniel Kiper]

The setup_data is a bit awkward to use for extremely large data objects,
both because the setup_data header has to be adjacent to the data object
and because it has a 32-bit length field. However, it is important that
intermediate stages of the boot process have a way to identify which
chunks of memory are occupied by kernel data. Thus we introduce an uniform
way to specify such indirect data as setup_indirect struct and
SETUP_INDIRECT type.

Suggested-by: H. Peter Anvin (Intel) 
Signed-off-by: Daniel Kiper 
Acked-by: Konrad Rzeszutek Wilk 
Reviewed-by: Ross Philipson 
Reviewed-by: H. Peter Anvin (Intel) 
---
v4 - suggestions/fixes:
   - change "Note:" to ".. note::".

v3 - suggestions/fixes:
   - add setup_indirect mapping/KASLR avoidance/etc. code
 (suggested by H. Peter Anvin),
   - the SETUP_INDIRECT sets most significant bit right now;
 this way it is possible to differentiate regular setup_data
 and setup_indirect objects in the debugfs filesystem.

v2 - suggestions/fixes:
   - add setup_indirect usage example
 (suggested by Eric Snowberg and Ross Philipson).
---
 Documentation/x86/boot.rst| 41 +++
 arch/x86/boot/compressed/kaslr.c  | 12 ++
 arch/x86/include/uapi/asm/bootparam.h | 16 +++---
 arch/x86/kernel/e820.c| 11 ++
 arch/x86/kernel/kdebugfs.c| 20 +
 arch/x86/kernel/ksysfs.c  | 30 +++--
 arch/x86/kernel/setup.c   |  4 
 arch/x86/mm/ioremap.c | 11 ++
 8 files changed, 131 insertions(+), 14 deletions(-)

diff --git a/Documentation/x86/boot.rst b/Documentation/x86/boot.rst
index 8e523c23ede3..38155ba8740f 100644
--- a/Documentation/x86/boot.rst
+++ b/Documentation/x86/boot.rst
@@ -827,6 +827,47 @@ Protocol:  2.09+
   sure to consider the case where the linked list already contains
   entries.
 
+  The setup_data is a bit awkward to use for extremely large data objects,
+  both because the setup_data header has to be adjacent to the data object
+  and because it has a 32-bit length field. However, it is important that
+  intermediate stages of the boot process have a way to identify which
+  chunks of memory are occupied by kernel data.
+
+  Thus setup_indirect struct and SETUP_INDIRECT type were introduced in
+  protocol 2.15.
+
+  struct setup_indirect {
+__u32 type;
+__u32 reserved;  /* Reserved, must be set to zero. */
+__u64 len;
+__u64 addr;
+  };
+
+  The type member is a SETUP_INDIRECT | SETUP_* type. However, it cannot be
+  SETUP_INDIRECT itself since making the setup_indirect a tree structure
+  could require a lot of stack space in something that needs to parse it
+  and stack space can be limited in boot contexts.
+
+  Let's give an example how to point to SETUP_E820_EXT data using 
setup_indirect.
+  In this case setup_data and setup_indirect will look like this:
+
+  struct setup_data {
+__u64 next = 0 or ;
+__u32 type = SETUP_INDIRECT;
+__u32 len = sizeof(setup_data);
+__u8 data[sizeof(setup_indirect)] = struct setup_indirect {
+  __u32 type = SETUP_INDIRECT | SETUP_E820_EXT;
+  __u32 reserved = 0;
+  __u64 len = ;
+  __u64 addr = ;
+}
+  }
+
+.. note::
+ SETUP_INDIRECT | SETUP_NONE objects cannot be properly distinguished
+ from SETUP_INDIRECT itself. So, this kind of objects cannot be provided
+ by the bootloaders.
+
    
 Field name:pref_address
 Type:  read (reloc)
diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index 2e53c056ba20..bb9bfef174ae 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -459,6 +459,18 @@ static bool mem_avoid_overlap(struct mem_vector *img,
is_overlapping = true;
}
 
+   if (ptr->type == SETUP_INDIRECT &&
+   ((struct setup_indirect *)ptr->data)->type != 
SETUP_INDIRECT) {
+   avoid.start = ((struct setup_indirect 
*)ptr->data)->addr;
+   avoid.size = ((struct setup_indirect *)ptr->data)->len;
+
+   if (mem_overlaps(img, ) && (avoid.start < 
earliest)) {
+   *overlap = avoid;
+   earliest = overlap->start;
+   is_overlapping = true;
+   }
+   }
+
ptr = (struct setup_data *)(unsigned long)ptr->next;
}
 
diff --git a/arch/x86/include/uapi/asm/bootparam.h 
b/arch/x86/include/uapi/asm/bootparam.h
index dbb41128e5a0..949066b5398a 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -2,7 +2,7 @@
 #ifndef _ASM_X86_BOOTPARAM_H
 #define _ASM_X86_BOOTPARAM_H
 
-/* setup_data types */
+/* setup_data/setup_indirect types */

[Xen-devel] [PATCH v4 2/3] x86/boot: Introduce the kernel_info.setup_type_max

[Daniel Kiper]

This field contains maximal allowed type for setup_data.

Now bump the setup_header version in arch/x86/boot/header.S.

Suggested-by: H. Peter Anvin (Intel) 
Signed-off-by: Daniel Kiper 
Reviewed-by: Konrad Rzeszutek Wilk 
Reviewed-by: Ross Philipson 
Reviewed-by: H. Peter Anvin (Intel) 
---
 Documentation/x86/boot.rst | 9 -
 arch/x86/boot/compressed/kernel_info.S | 5 +
 arch/x86/boot/header.S | 2 +-
 arch/x86/include/uapi/asm/bootparam.h  | 3 +++
 4 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/Documentation/x86/boot.rst b/Documentation/x86/boot.rst
index c60fafda9427..8e523c23ede3 100644
--- a/Documentation/x86/boot.rst
+++ b/Documentation/x86/boot.rst
@@ -73,7 +73,7 @@ Protocol 2.14:BURNT BY INCORRECT COMMIT 
ae7e1238e68f2a472a125673ab506d49158c188
(x86/boot: Add ACPI RSDP address to setup_header)
DO NOT USE!!! ASSUME SAME AS 2.13.
 
-Protocol 2.15: (Kernel 5.5) Added the kernel_info.
+Protocol 2.15: (Kernel 5.5) Added the kernel_info and 
kernel_info.setup_type_max.
 =  
 
 .. note::
@@ -981,6 +981,13 @@ Offset/size:   0x0008/4
   This field contains the size of the kernel_info including kernel_info.header
   and kernel_info.kernel_info_var_len_data.
 
+   ==
+Field name:setup_type_max
+Offset/size:   0x0008/4
+   ==
+
+  This field contains maximal allowed type for setup_data and setup_indirect 
structs.
+
 
 The Image Checksum
 ==
diff --git a/arch/x86/boot/compressed/kernel_info.S 
b/arch/x86/boot/compressed/kernel_info.S
index 8ea6f6e3feef..f818ee8fba38 100644
--- a/arch/x86/boot/compressed/kernel_info.S
+++ b/arch/x86/boot/compressed/kernel_info.S
@@ -1,5 +1,7 @@
 /* SPDX-License-Identifier: GPL-2.0 */
 
+#include 
+
.section ".rodata.kernel_info", "a"
 
.global kernel_info
@@ -12,6 +14,9 @@ kernel_info:
/* Size total. */
.long   kernel_info_end - kernel_info
 
+   /* Maximal allowed type for setup_data and setup_indirect structs. */
+   .long   SETUP_TYPE_MAX
+
 kernel_info_var_len_data:
/* Empty for time being... */
 kernel_info_end:
diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
index 22dcecaaa898..97d9b6d6c1af 100644
--- a/arch/x86/boot/header.S
+++ b/arch/x86/boot/header.S
@@ -300,7 +300,7 @@ _start:
# Part 2 of the header, from the old setup.S
 
.ascii  "HdrS"  # header signature
-   .word   0x020d  # header version number (>= 0x0105)
+   .word   0x020f  # header version number (>= 0x0105)
# or else old loadlin-1.5 will fail)
.globl realmode_swtch
 realmode_swtch:.word   0, 0# default_switch, SETUPSEG
diff --git a/arch/x86/include/uapi/asm/bootparam.h 
b/arch/x86/include/uapi/asm/bootparam.h
index a1ebcd7a991c..dbb41128e5a0 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -11,6 +11,9 @@
 #define SETUP_APPLE_PROPERTIES 5
 #define SETUP_JAILHOUSE6
 
+/* max(SETUP_*) */
+#define SETUP_TYPE_MAX SETUP_JAILHOUSE
+
 /* ram_size flags */
 #define RAMDISK_IMAGE_START_MASK   0x07FF
 #define RAMDISK_PROMPT_FLAG0x8000
-- 
2.11.0


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

[Xen-devel] [PATCH v4 1/3] x86/boot: Introduce the kernel_info

[Daniel Kiper]

The relationships between the headers are analogous to the various data
sections:

  setup_header = .data
  boot_params/setup_data = .bss

What is missing from the above list? That's right:

  kernel_info = .rodata

We have been (ab)using .data for things that could go into .rodata or .bss for
a long time, for lack of alternatives and -- especially early on -- inertia.
Also, the BIOS stub is responsible for creating boot_params, so it isn't
available to a BIOS-based loader (setup_data is, though).

setup_header is permanently limited to 144 bytes due to the reach of the
2-byte jump field, which doubles as a length field for the structure, combined
with the size of the "hole" in struct boot_params that a protected-mode loader
or the BIOS stub has to copy it into. It is currently 119 bytes long, which
leaves us with 25 very precious bytes. This isn't something that can be fixed
without revising the boot protocol entirely, breaking backwards compatibility.

boot_params proper is limited to 4096 bytes, but can be arbitrarily extended
by adding setup_data entries. It cannot be used to communicate properties of
the kernel image, because it is .bss and has no image-provided content.

kernel_info solves this by providing an extensible place for information about
the kernel image. It is readonly, because the kernel cannot rely on a
bootloader copying its contents anywhere, but that is OK; if it becomes
necessary it can still contain data items that an enabled bootloader would be
expected to copy into a setup_data chunk.

This patch does not bump setup_header version in arch/x86/boot/header.S
because it will be followed by additional changes coming into the
Linux/x86 boot protocol.

Suggested-by: H. Peter Anvin (Intel) 
Signed-off-by: Daniel Kiper 
Reviewed-by: Konrad Rzeszutek Wilk 
Reviewed-by: Ross Philipson 
Reviewed-by: H. Peter Anvin (Intel) 
---
v4 - suggestions/fixes:
   - improve the documentation
 (suggested by Randy Dunlap and Konrad Rzeszutek Wilk).

v3 - suggestions/fixes:
   - split kernel_info data into fixed and variable sized regions,
 (suggested by H. Peter Anvin),
   - change kernel_info.header value to "LToP" (0x506f544c),
 (suggested by H. Peter Anvin),
   - improve the comments,
   - improve the documentation.

v2 - suggestions/fixes:
   - rename setup_header2 to kernel_info,
 (suggested by H. Peter Anvin),
   - change kernel_info.header value to "InfO" (0x4f666e49),
   - new kernel_info description in Documentation/x86/boot.rst,
 (suggested by H. Peter Anvin),
   - drop kernel_info_offset_update() as an overkill and
 update kernel_info offset directly from main(),
 (suggested by Eric Snowberg),
   - new commit message
 (suggested by H. Peter Anvin),
   - fix some commit message misspellings
 (suggested by Eric Snowberg).
---
 Documentation/x86/boot.rst | 126 +
 arch/x86/boot/Makefile |   2 +-
 arch/x86/boot/compressed/Makefile  |   4 +-
 arch/x86/boot/compressed/kernel_info.S |  17 +
 arch/x86/boot/header.S |   1 +
 arch/x86/boot/tools/build.c|   5 ++
 arch/x86/include/uapi/asm/bootparam.h  |   1 +
 7 files changed, 153 insertions(+), 3 deletions(-)
 create mode 100644 arch/x86/boot/compressed/kernel_info.S

diff --git a/Documentation/x86/boot.rst b/Documentation/x86/boot.rst
index 08a2f100c0e6..c60fafda9427 100644
--- a/Documentation/x86/boot.rst
+++ b/Documentation/x86/boot.rst
@@ -68,8 +68,25 @@ Protocol 2.12(Kernel 3.8) Added the xloadflags field 
and extension fields
 Protocol 2.13  (Kernel 3.14) Support 32- and 64-bit flags being set in
xloadflags to support booting a 64-bit kernel from 32-bit
EFI
+
+Protocol 2.14: BURNT BY INCORRECT COMMIT 
ae7e1238e68f2a472a125673ab506d49158c1889
+   (x86/boot: Add ACPI RSDP address to setup_header)
+   DO NOT USE!!! ASSUME SAME AS 2.13.
+
+Protocol 2.15: (Kernel 5.5) Added the kernel_info.
 =  
 
+.. note::
+ The protocol version number should be changed only if the setup header
+ is changed. There is no need to update the version number if boot_params
+ or kernel_info are changed. Additionally, it is recommended to use
+ xloadflags (in this case the protocol version number should not be
+ updated either) or kernel_info to communicate supported Linux kernel
+ features to the boot loader. Due to very limited space available in
+ the original setup header every update to it should be considered
+ with great care. Starting from the protocol 2.15 the primary way to
+ communicate things to the boot loader is the kernel_info.
+
 
 Memory Layout
 =
@@ -207,6 +224,7 @@ Offset/Size Proto   NameMeaning
 0258/8 2.10+   pref_addressPreferred loading

[Xen-devel] [PATCH v4 0/3] x86/boot: Introduce the kernel_info et consortes

[Daniel Kiper]

Hi,

Due to very limited space in the setup_header this patch series introduces new
kernel_info struct which will be used to convey information from the kernel to
the bootloader. This way the boot protocol can be extended regardless of the
setup_header limitations. Additionally, the patch series introduces some
convenience features like the setup_indirect struct and the
kernel_info.setup_type_max field.

Daniel

 Documentation/x86/boot.rst | 174 
++
 arch/x86/boot/Makefile |   2 +-
 arch/x86/boot/compressed/Makefile  |   4 +-
 arch/x86/boot/compressed/kaslr.c   |  12 ++
 arch/x86/boot/compressed/kernel_info.S |  22 ++
 arch/x86/boot/header.S |   3 +-
 arch/x86/boot/tools/build.c|   5 +++
 arch/x86/include/uapi/asm/bootparam.h  |  16 +++-
 arch/x86/kernel/e820.c |  11 +
 arch/x86/kernel/kdebugfs.c |  20 +++--
 arch/x86/kernel/ksysfs.c   |  30 ++
 arch/x86/kernel/setup.c|   4 ++
 arch/x86/mm/ioremap.c  |  11 +
 13 files changed, 298 insertions(+), 16 deletions(-)

Daniel Kiper (3):
  x86/boot: Introduce the kernel_info
  x86/boot: Introduce the kernel_info.setup_type_max
  x86/boot: Introduce the setup_indirect


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

Re: [Xen-devel] [PATCH v3 0/3] x86/boot: Introduce the kernel_info et consortes

[Daniel Kiper]

On Wed, Oct 09, 2019 at 12:53:55PM +0200, Daniel Kiper wrote:
> Hi,
>
> Due to very limited space in the setup_header this patch series introduces new
> kernel_info struct which will be used to convey information from the kernel to
> the bootloader. This way the boot protocol can be extended regardless of the
> setup_header limitations. Additionally, the patch series introduces some
> convenience features like the setup_indirect struct and the
> kernel_info.setup_type_max field.
>
> Daniel
>
>  Documentation/x86/boot.rst | 168 
> ++
>  arch/x86/boot/Makefile |   2 +-
>  arch/x86/boot/compressed/Makefile  |   4 +-
>  arch/x86/boot/compressed/kaslr.c   |  12 ++
>  arch/x86/boot/compressed/kernel_info.S |  22 +++
>  arch/x86/boot/header.S |   3 +-
>  arch/x86/boot/tools/build.c|   5 +++
>  arch/x86/include/uapi/asm/bootparam.h  |  16 +++-
>  arch/x86/kernel/e820.c |  11 ++
>  arch/x86/kernel/kdebugfs.c |  20 --
>  arch/x86/kernel/ksysfs.c   |  30 ++
>  arch/x86/kernel/setup.c|   4 ++
>  arch/x86/mm/ioremap.c  |  11 ++
>  13 files changed, 292 insertions(+), 16 deletions(-)
>
> Daniel Kiper (3):
>   x86/boot: Introduce the kernel_info
>   x86/boot: Introduce the kernel_info.setup_type_max
>   x86/boot: Introduce the setup_indirect

hpa, ping?

Daniel

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

Re: [Xen-devel] [PATCH v3 1/3] x86/boot: Introduce the kernel_info

[Daniel Kiper]

On Wed, Oct 09, 2019 at 05:43:31PM -0700, Randy Dunlap wrote:
> Hi,
>
> Questions and comments below...
> Thanks.
>
> On 10/9/19 3:53 AM, Daniel Kiper wrote:
>
> > Suggested-by: H. Peter Anvin 
> > Signed-off-by: Daniel Kiper 
> > Reviewed-by: Konrad Rzeszutek Wilk 
> > Reviewed-by: Ross Philipson 
> > ---
>
> > ---
> >  Documentation/x86/boot.rst | 121 
> > +
> >  arch/x86/boot/Makefile |   2 +-
> >  arch/x86/boot/compressed/Makefile  |   4 +-
> >  arch/x86/boot/compressed/kernel_info.S |  17 +
> >  arch/x86/boot/header.S |   1 +
> >  arch/x86/boot/tools/build.c|   5 ++
> >  arch/x86/include/uapi/asm/bootparam.h  |   1 +
> >  7 files changed, 148 insertions(+), 3 deletions(-)
> >  create mode 100644 arch/x86/boot/compressed/kernel_info.S
> >
> > diff --git a/Documentation/x86/boot.rst b/Documentation/x86/boot.rst
> > index 08a2f100c0e6..d5323a39f5e3 100644
> > --- a/Documentation/x86/boot.rst
> > +++ b/Documentation/x86/boot.rst
> > @@ -68,8 +68,25 @@ Protocol 2.12(Kernel 3.8) Added the xloadflags field 
> > and extension fields
> >  Protocol 2.13  (Kernel 3.14) Support 32- and 64-bit flags being set in
> > xloadflags to support booting a 64-bit kernel from 32-bit
> > EFI
> > +
> > +Protocol 2.14: BURNT BY INCORRECT COMMIT 
> > ae7e1238e68f2a472a125673ab506d49158c1889
> > +   (x86/boot: Add ACPI RSDP address to setup_header)
> > +   DO NOT USE!!! ASSUME SAME AS 2.13.
> > +
> > +Protocol 2.15: (Kernel 5.5) Added the kernel_info.
> >  =  
> > 
> >
> > +.. note::
> > + The protocol version number should be changed only if the setup header
> > + is changed. There is no need to update the version number if 
> > boot_params
> > + or kernel_info are changed. Additionally, it is recommended to use
> > + xloadflags (in this case the protocol version number should not be
> > + updated either) or kernel_info to communicate supported Linux kernel
> > + features to the boot loader. Due to very limited space available in
> > + the original setup header every update to it should be considered
> > + with great care. Starting from the protocol 2.15 the primary way to
> > + communicate things to the boot loader is the kernel_info.
> > +
> >
> >  Memory Layout
> >  =
> > @@ -207,6 +224,7 @@ Offset/Size Proto   Name
> > Meaning
> >  0258/8 2.10+   pref_addressPreferred 
> > loading address
> >  0260/4 2.10+   init_size   Linear memory 
> > required during initialization
> >  0264/4 2.11+   handover_offset Offset of 
> > handover entry point
> > +0268/4 2.15+   kernel_info_offset  Offset of the 
> > kernel_info
> >  ====   
> > 
> >
> >  .. note::
> > @@ -855,6 +873,109 @@ Offset/size:  0x264/4
> >
> >See EFI HANDOVER PROTOCOL below for more details.
> >
> > +   ==
> > +Field name:kernel_info_offset
> > +Type:  read
> > +Offset/size:   0x268/4
> > +Protocol:  2.15+
> > +   ==
> > +
> > +  This field is the offset from the beginning of the kernel image to the
> > +  kernel_info. It is embedded in the Linux image in the uncompressed
>   ^^
>What does  It   refer to, please?

s/It/The kernel_info structure/ Is it better?

> > +  protected mode region.
> > +
> > +
> > +The kernel_info
> > +===
> > +
> > +The relationships between the headers are analogous to the various data
> > +sections:
> > +
> > +  setup_header = .data
> > +  boot_params/setup_data = .bss
> > +
> > +What is missing from the above list? That's right:
> > +
> > +  kernel_info = .rodata
> > +
> > +We have been (ab)using .data for things that could go into .rodata or .bss 
> > for
> > +a long time, for lack of alternatives and -- especially early on -- 
> > inertia.
> > +Also, the BIOS stub is responsible for creating boot_params, so it isn't
> > +available to a BIOS-based loader (setup_data is, though

[Xen-devel] [PATCH v3 0/3] x86/boot: Introduce the kernel_info et consortes

[Daniel Kiper]

Hi,

Due to very limited space in the setup_header this patch series introduces new
kernel_info struct which will be used to convey information from the kernel to
the bootloader. This way the boot protocol can be extended regardless of the
setup_header limitations. Additionally, the patch series introduces some
convenience features like the setup_indirect struct and the
kernel_info.setup_type_max field.

Daniel

 Documentation/x86/boot.rst | 168 
++
 arch/x86/boot/Makefile |   2 +-
 arch/x86/boot/compressed/Makefile  |   4 +-
 arch/x86/boot/compressed/kaslr.c   |  12 ++
 arch/x86/boot/compressed/kernel_info.S |  22 +++
 arch/x86/boot/header.S |   3 +-
 arch/x86/boot/tools/build.c|   5 +++
 arch/x86/include/uapi/asm/bootparam.h  |  16 +++-
 arch/x86/kernel/e820.c |  11 ++
 arch/x86/kernel/kdebugfs.c |  20 --
 arch/x86/kernel/ksysfs.c   |  30 ++
 arch/x86/kernel/setup.c|   4 ++
 arch/x86/mm/ioremap.c  |  11 ++
 13 files changed, 292 insertions(+), 16 deletions(-)

Daniel Kiper (3):
  x86/boot: Introduce the kernel_info
  x86/boot: Introduce the kernel_info.setup_type_max
  x86/boot: Introduce the setup_indirect


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

[Xen-devel] [PATCH v3 2/3] x86/boot: Introduce the kernel_info.setup_type_max

[Daniel Kiper]

This field contains maximal allowed type for setup_data.

Now bump the setup_header version in arch/x86/boot/header.S.

Suggested-by: H. Peter Anvin 
Signed-off-by: Daniel Kiper 
Reviewed-by: Konrad Rzeszutek Wilk 
Reviewed-by: Ross Philipson 
---
 Documentation/x86/boot.rst | 9 -
 arch/x86/boot/compressed/kernel_info.S | 5 +
 arch/x86/boot/header.S | 2 +-
 arch/x86/include/uapi/asm/bootparam.h  | 3 +++
 4 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/Documentation/x86/boot.rst b/Documentation/x86/boot.rst
index d5323a39f5e3..4c536bc8816d 100644
--- a/Documentation/x86/boot.rst
+++ b/Documentation/x86/boot.rst
@@ -73,7 +73,7 @@ Protocol 2.14:BURNT BY INCORRECT COMMIT 
ae7e1238e68f2a472a125673ab506d49158c188
(x86/boot: Add ACPI RSDP address to setup_header)
DO NOT USE!!! ASSUME SAME AS 2.13.
 
-Protocol 2.15: (Kernel 5.5) Added the kernel_info.
+Protocol 2.15: (Kernel 5.5) Added the kernel_info and 
kernel_info.setup_type_max.
 =  
 
 .. note::
@@ -976,6 +976,13 @@ Offset/size:   0x0008/4
   This field contains the size of the kernel_info including kernel_info.header
   and kernel_info.kernel_info_var_len_data.
 
+   ==
+Field name:setup_type_max
+Offset/size:   0x0008/4
+   ==
+
+  This field contains maximal allowed type for setup_data and setup_indirect 
structs.
+
 
 The Image Checksum
 ==
diff --git a/arch/x86/boot/compressed/kernel_info.S 
b/arch/x86/boot/compressed/kernel_info.S
index 8ea6f6e3feef..f818ee8fba38 100644
--- a/arch/x86/boot/compressed/kernel_info.S
+++ b/arch/x86/boot/compressed/kernel_info.S
@@ -1,5 +1,7 @@
 /* SPDX-License-Identifier: GPL-2.0 */
 
+#include 
+
.section ".rodata.kernel_info", "a"
 
.global kernel_info
@@ -12,6 +14,9 @@ kernel_info:
/* Size total. */
.long   kernel_info_end - kernel_info
 
+   /* Maximal allowed type for setup_data and setup_indirect structs. */
+   .long   SETUP_TYPE_MAX
+
 kernel_info_var_len_data:
/* Empty for time being... */
 kernel_info_end:
diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
index 22dcecaaa898..97d9b6d6c1af 100644
--- a/arch/x86/boot/header.S
+++ b/arch/x86/boot/header.S
@@ -300,7 +300,7 @@ _start:
# Part 2 of the header, from the old setup.S
 
.ascii  "HdrS"  # header signature
-   .word   0x020d  # header version number (>= 0x0105)
+   .word   0x020f  # header version number (>= 0x0105)
# or else old loadlin-1.5 will fail)
.globl realmode_swtch
 realmode_swtch:.word   0, 0# default_switch, SETUPSEG
diff --git a/arch/x86/include/uapi/asm/bootparam.h 
b/arch/x86/include/uapi/asm/bootparam.h
index a1ebcd7a991c..dbb41128e5a0 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -11,6 +11,9 @@
 #define SETUP_APPLE_PROPERTIES 5
 #define SETUP_JAILHOUSE6
 
+/* max(SETUP_*) */
+#define SETUP_TYPE_MAX SETUP_JAILHOUSE
+
 /* ram_size flags */
 #define RAMDISK_IMAGE_START_MASK   0x07FF
 #define RAMDISK_PROMPT_FLAG0x8000
-- 
2.11.0


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

[Xen-devel] [PATCH v3 3/3] x86/boot: Introduce the setup_indirect

[Daniel Kiper]

The setup_data is a bit awkward to use for extremely large data objects,
both because the setup_data header has to be adjacent to the data object
and because it has a 32-bit length field. However, it is important that
intermediate stages of the boot process have a way to identify which
chunks of memory are occupied by kernel data. Thus we introduce an uniform
way to specify such indirect data as setup_indirect struct and
SETUP_INDIRECT type.

Suggested-by: H. Peter Anvin 
Signed-off-by: Daniel Kiper 
Acked-by: Konrad Rzeszutek Wilk 
Reviewed-by: Ross Philipson 
---
v3 - suggestions/fixes:
   - add setup_indirect mapping/KASLR avoidance/etc. code
 (suggested by H. Peter Anvin),
   - the SETUP_INDIRECT sets most significant bit right now;
 this way it is possible to differentiate regular setup_data
 and setup_indirect objects in the debugfs filesystem.

v2 - suggestions/fixes:
   - add setup_indirect usage example
 (suggested by Eric Snowberg and Ross Philipson).
---
 Documentation/x86/boot.rst| 40 +++
 arch/x86/boot/compressed/kaslr.c  | 12 +++
 arch/x86/include/uapi/asm/bootparam.h | 16 +++---
 arch/x86/kernel/e820.c| 11 ++
 arch/x86/kernel/kdebugfs.c| 20 ++
 arch/x86/kernel/ksysfs.c  | 30 --
 arch/x86/kernel/setup.c   |  4 
 arch/x86/mm/ioremap.c | 11 ++
 8 files changed, 130 insertions(+), 14 deletions(-)

diff --git a/Documentation/x86/boot.rst b/Documentation/x86/boot.rst
index 4c536bc8816d..d6d03b00b594 100644
--- a/Documentation/x86/boot.rst
+++ b/Documentation/x86/boot.rst
@@ -827,6 +827,46 @@ Protocol:  2.09+
   sure to consider the case where the linked list already contains
   entries.
 
+  The setup_data is a bit awkward to use for extremely large data objects,
+  both because the setup_data header has to be adjacent to the data object
+  and because it has a 32-bit length field. However, it is important that
+  intermediate stages of the boot process have a way to identify which
+  chunks of memory are occupied by kernel data.
+
+  Thus setup_indirect struct and SETUP_INDIRECT type were introduced in
+  protocol 2.15.
+
+  struct setup_indirect {
+__u32 type;
+__u32 reserved;  /* Reserved, must be set to zero. */
+__u64 len;
+__u64 addr;
+  };
+
+  The type member is a SETUP_INDIRECT | SETUP_* type. However, it cannot be
+  SETUP_INDIRECT itself since making the setup_indirect a tree structure
+  could require a lot of stack space in something that needs to parse it
+  and stack space can be limited in boot contexts.
+
+  Let's give an example how to point to SETUP_E820_EXT data using 
setup_indirect.
+  In this case setup_data and setup_indirect will look like this:
+
+  struct setup_data {
+__u64 next = 0 or ;
+__u32 type = SETUP_INDIRECT;
+__u32 len = sizeof(setup_data);
+__u8 data[sizeof(setup_indirect)] = struct setup_indirect {
+  __u32 type = SETUP_INDIRECT | SETUP_E820_EXT;
+  __u32 reserved = 0;
+  __u64 len = ;
+  __u64 addr = ;
+}
+  }
+
+  Note: SETUP_INDIRECT | SETUP_NONE objects cannot be properly distinguished
+from SETUP_INDIRECT itself. So, this kind of objects cannot be provided
+by the bootloaders.
+
    
 Field name:pref_address
 Type:  read (reloc)
diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index 2e53c056ba20..bb9bfef174ae 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -459,6 +459,18 @@ static bool mem_avoid_overlap(struct mem_vector *img,
is_overlapping = true;
}
 
+   if (ptr->type == SETUP_INDIRECT &&
+   ((struct setup_indirect *)ptr->data)->type != 
SETUP_INDIRECT) {
+   avoid.start = ((struct setup_indirect 
*)ptr->data)->addr;
+   avoid.size = ((struct setup_indirect *)ptr->data)->len;
+
+   if (mem_overlaps(img, ) && (avoid.start < 
earliest)) {
+   *overlap = avoid;
+   earliest = overlap->start;
+   is_overlapping = true;
+   }
+   }
+
ptr = (struct setup_data *)(unsigned long)ptr->next;
}
 
diff --git a/arch/x86/include/uapi/asm/bootparam.h 
b/arch/x86/include/uapi/asm/bootparam.h
index dbb41128e5a0..949066b5398a 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -2,7 +2,7 @@
 #ifndef _ASM_X86_BOOTPARAM_H
 #define _ASM_X86_BOOTPARAM_H
 
-/* setup_data types */
+/* setup_data/setup_indirect types */
 #define SETUP_NONE 0
 #define SETUP_E820_EXT 1
 #define SETUP_DTB 

[Xen-devel] [PATCH v3 1/3] x86/boot: Introduce the kernel_info

[Daniel Kiper]

The relationships between the headers are analogous to the various data
sections:

  setup_header = .data
  boot_params/setup_data = .bss

What is missing from the above list? That's right:

  kernel_info = .rodata

We have been (ab)using .data for things that could go into .rodata or .bss for
a long time, for lack of alternatives and -- especially early on -- inertia.
Also, the BIOS stub is responsible for creating boot_params, so it isn't
available to a BIOS-based loader (setup_data is, though).

setup_header is permanently limited to 144 bytes due to the reach of the
2-byte jump field, which doubles as a length field for the structure, combined
with the size of the "hole" in struct boot_params that a protected-mode loader
or the BIOS stub has to copy it into. It is currently 119 bytes long, which
leaves us with 25 very precious bytes. This isn't something that can be fixed
without revising the boot protocol entirely, breaking backwards compatibility.

boot_params proper is limited to 4096 bytes, but can be arbitrarily extended
by adding setup_data entries. It cannot be used to communicate properties of
the kernel image, because it is .bss and has no image-provided content.

kernel_info solves this by providing an extensible place for information about
the kernel image. It is readonly, because the kernel cannot rely on a
bootloader copying its contents anywhere, but that is OK; if it becomes
necessary it can still contain data items that an enabled bootloader would be
expected to copy into a setup_data chunk.

This patch does not bump setup_header version in arch/x86/boot/header.S
because it will be followed by additional changes coming into the
Linux/x86 boot protocol.

Suggested-by: H. Peter Anvin 
Signed-off-by: Daniel Kiper 
Reviewed-by: Konrad Rzeszutek Wilk 
Reviewed-by: Ross Philipson 
---
v3 - suggestions/fixes:
   - split kernel_info data into fixed and variable sized regions,
 (suggested by H. Peter Anvin),
   - change kernel_info.header value to "LToP" (0x506f544c),
 (suggested by H. Peter Anvin),
   - improve the comments,
   - improve the documentation.

v2 - suggestions/fixes:
   - rename setup_header2 to kernel_info,
 (suggested by H. Peter Anvin),
   - change kernel_info.header value to "InfO" (0x4f666e49),
   - new kernel_info description in Documentation/x86/boot.rst,
 (suggested by H. Peter Anvin),
   - drop kernel_info_offset_update() as an overkill and
 update kernel_info offset directly from main(),
 (suggested by Eric Snowberg),
   - new commit message
 (suggested by H. Peter Anvin),
   - fix some commit message misspellings
 (suggested by Eric Snowberg).
---
 Documentation/x86/boot.rst | 121 +
 arch/x86/boot/Makefile |   2 +-
 arch/x86/boot/compressed/Makefile  |   4 +-
 arch/x86/boot/compressed/kernel_info.S |  17 +
 arch/x86/boot/header.S |   1 +
 arch/x86/boot/tools/build.c|   5 ++
 arch/x86/include/uapi/asm/bootparam.h  |   1 +
 7 files changed, 148 insertions(+), 3 deletions(-)
 create mode 100644 arch/x86/boot/compressed/kernel_info.S

diff --git a/Documentation/x86/boot.rst b/Documentation/x86/boot.rst
index 08a2f100c0e6..d5323a39f5e3 100644
--- a/Documentation/x86/boot.rst
+++ b/Documentation/x86/boot.rst
@@ -68,8 +68,25 @@ Protocol 2.12(Kernel 3.8) Added the xloadflags field 
and extension fields
 Protocol 2.13  (Kernel 3.14) Support 32- and 64-bit flags being set in
xloadflags to support booting a 64-bit kernel from 32-bit
EFI
+
+Protocol 2.14: BURNT BY INCORRECT COMMIT 
ae7e1238e68f2a472a125673ab506d49158c1889
+   (x86/boot: Add ACPI RSDP address to setup_header)
+   DO NOT USE!!! ASSUME SAME AS 2.13.
+
+Protocol 2.15: (Kernel 5.5) Added the kernel_info.
 =  
 
+.. note::
+ The protocol version number should be changed only if the setup header
+ is changed. There is no need to update the version number if boot_params
+ or kernel_info are changed. Additionally, it is recommended to use
+ xloadflags (in this case the protocol version number should not be
+ updated either) or kernel_info to communicate supported Linux kernel
+ features to the boot loader. Due to very limited space available in
+ the original setup header every update to it should be considered
+ with great care. Starting from the protocol 2.15 the primary way to
+ communicate things to the boot loader is the kernel_info.
+
 
 Memory Layout
 =
@@ -207,6 +224,7 @@ Offset/Size Proto   NameMeaning
 0258/8 2.10+   pref_addressPreferred loading 
address
 0260/4 2.10+   init_size   Linear memory required 
during initialization
 0264/4 2.11+   handover_offset Offset of

Re: [Xen-devel] [PATCH 3/4] x86/linker: add a reloc section to ELF binary

[Daniel Kiper]

On Fri, Jun 21, 2019 at 12:34:13AM -0600, Jan Beulich wrote:
> >>> On 19.06.19 at 17:06,  wrote:
> > On Wed, Jun 19, 2019 at 06:57:05AM -0600, Jan Beulich wrote:
> >> >>> On 19.06.19 at 13:02,  wrote:
> >> > If the hypervisor has been built with EFI support (ie: multiboot2).
> >> > This allows to position the .reloc section correctly in the output
> >> > binary, or else the linker might place .reloc before the .text
> >> > section.
> >> >
> >> > Note that the .reloc section is moved before .bss for two reasons: in
> >> > order for the resulting binary to not contain any section with data
> >> > after .bss, so that the file size can be smaller than the loaded
> >> > memory size, and because the data it contains is read-only, so it
> >> > belongs with the other sections containing read-only data.
> >>
> >> While this may be fine for ELF, I'm afraid it would be calling for
> >> subtle issues with xen.efi (i.e. the PE binary): There a .reloc
> >> section is generally expected to come after "normal" data
> >> sections.
> >
> > OK, would you like me to leave the .reloc section at the previous
> > position for EFI builds then?
>
> Well, this part is a requirement, not a question of me liking you
> to do so.
>
> > Or do we prefer to leave .reloc orphaned in the ELF build?
>
> Daniel might have an opinion here with his plans to actually
> add relocations there in the non-linker-generated-PE build. I
> don't have a strong opinion either way, as long as the
> current method of building gets left as is (or even simplified).

I would not drop .reloc section from xen-syms because it can be useful
for "manual" EFI image relocs generation. However, I am not strongly
tied to it. If you wish to drop it go ahead. I can readd it latter if
I get back to my new PE build work.

Daniel

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

Re: [Xen-devel] Fedora - make BLS configs default - Xen Dom0 boot broken

[Daniel Kiper]

On Wed, May 15, 2019 at 02:02:57PM +0100, M A Young wrote:
> On Wed, 15 May 2019, Daniel Kiper wrote:
>
> > FYI, another Xen Dom0 boot issue on Fedora...
> >
> > Please take a look at [1]. This will break Xen Dom0 boot due to lack of
> > support for multiboot, multiboot2, module, and module2 commands. If we
> > care then this has to be fixed somehow...
>
> This bit isn't currently a problem for Xen Dom0 because the
> /etc/grub.d/20_linux_xen grub script still generates non-BLS grub
> configuration.

Great! So, this makes the issue not so much urgent but we have to have
it on our radar.

Daniel

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

[Xen-devel] Fedora - make BLS configs default - Xen Dom0 boot broken

[Daniel Kiper]

Hey,

FYI, another Xen Dom0 boot issue on Fedora...

Please take a look at [1]. This will break Xen Dom0 boot due to lack of
support for multiboot, multiboot2, module, and module2 commands. If we
care then this has to be fixed somehow...

Again, I can coordinate work and review patches but I cannot take a stub
at the issue myself. Sorry about that.

Daniel

[1] https://fedoraproject.org/wiki/Changes/BootLoaderSpecByDefault

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

Re: [Xen-devel] Criteria / validation proposal: drop Xen

[Daniel Kiper]

I have not heard that Peter moved to the Oracle ;-))), so, changing his
address back to RedHat one. And dropping Fedora testing mailing list
address because I do not have permissions to send to it...

On Mon, Apr 29, 2019 at 10:51:47AM +0200, Daniel Kiper wrote:
> Sorry for top posting...
>
> FYI, I am on vacation right now. I will be back next week. So, if it is
> not urgent I will explain all the stuff and update relevant bug at the
> beginning of next week. Sorry for delay.
>
> Daniel
>
> On Sun, Apr 28, 2019 at 12:44:37PM +1000, Steven Haigh wrote:
> > (and sending to the list this time due to Geary being rather featureless
> > mail client)
> >
> > As one of those being caught by regressions upgrading F29 to F30 under Xen
> > DomU's, I think this is a bad idea.
> >
> > It shows that it wasn't tested, because it doesn't work. To me, this exposes
> > weaknesses in the testing and the solution shouldn't be "The check fails,
> > remove the check".
> >
> > On Sat, Apr 27, 2019 at 4:18 AM, Konrad Rzeszutek Wilk
> >  wrote:
> > > On Fri, Apr 26, 2019 at 10:22:13PM +0530, Sumantro Mukherjee wrote:
> > > >  Yup +1 from my side too. Xen is hardly tested since a lot of time.
> > >
> > > Hi!
> > >
> > > And that is thanks to one of the GRUB2 bugs that needs some love
> > > from Peter Jones.
> > >
> > > As without that bug being fixed - it is very difficult to test it - as
> > > you can't even load Xen!
> > >
> > > I've asked the upstream GRUB maintainer to sheed some light on the
> > > confusion about multiboot2 + SecureBoot - hopefully that will resolve
> > > the question.
> > >
> > > My vote is to have it remain as is.
> > >
> > > Thank you.
> > > >
> > > >  On Fri, Apr 26, 2019 at 10:07 PM Geoffrey Marr 
> > > > wrote:
> > > >
> > > >  > Since F24, I haven't seen or heard of anyone who uses Xen over KVM
> > > >  > anywhere other than this thread... I'm +1 for making this test an
> > > >  > "Optional" one.
> > > >  >
> > > >  > Geoff Marr
> > > >  > IRC: coremodule
> > > >  >
> > > >  >
> > > >  > On Fri, Apr 26, 2019 at 10:33 AM Adam Williamson <
> > > >  > adamw...@fedoraproject.org> wrote:
> > > >  >
> > > >  >> On Thu, 2017-07-06 at 13:19 -0700, Adam Williamson wrote:
> > > >  >> > On Thu, 2017-07-06 at 15:59 -0400, Konrad Rzeszutek Wilk wrote:
> > > >  >> > > > > I would prefer for it to remain as it is.
> > > >  >> > > >
> > > >  >> > > > This is only practical if it's going to be tested, and
> > > > tested
> > > >  >> regularly
> > > >  >> > > > - not *only* on the final release candidate, right before
> > > > we sign
> > > >  >> off
> > > >  >> > > > on the release. It needs to be tested regularly throughout
> > > > the
> > > >  >> release
> > > >  >> > > > cycle, on the composes that are "nominated for testing".
> > > >  >> > >
> > > >  >> > > Right, which is why I am happy that you have pointed me to
> > > > the right
> > > >  >> > > place so I can be up-to-date.
> > > >  >> >
> > > >  >> > Great, thanks. So let's leave it as it is for now, but we'll
> > > > keep an
> > > >  >> > eye on this during F27 cycle. If we get to, say, Beta and
> > > > there are no
> > > >  >> > results for the test, that's gonna be a problem. Thanks!
> > > >  >>
> > > >  >> So, for Fedora 30, this was not tested throughout the whole
> > > > cycle. I
> > > >  >> think we can consider the proposal to remove the criterion active
> > > >  >> again.
> > > >  >> --
> > > >  >> Adam Williamson
> > > >  >> Fedora QA Community Monkey
> > > >  >> IRC: adamw | Twitter: AdamW_Fedora | XMPP: adamw AT
> > > > happyassassin . net
> > > >  >> http://www.happyassassin.net
> > > >  >> ___
> > > >  >> test mailing list -- t...@lists.fedoraproject.org
> > > >  

Re: [Xen-devel] Criteria / validation proposal: drop Xen

[Daniel Kiper]

Sorry for top posting...

FYI, I am on vacation right now. I will be back next week. So, if it is
not urgent I will explain all the stuff and update relevant bug at the
beginning of next week. Sorry for delay.

Daniel

On Sun, Apr 28, 2019 at 12:44:37PM +1000, Steven Haigh wrote:
> (and sending to the list this time due to Geary being rather featureless
> mail client)
>
> As one of those being caught by regressions upgrading F29 to F30 under Xen
> DomU's, I think this is a bad idea.
>
> It shows that it wasn't tested, because it doesn't work. To me, this exposes
> weaknesses in the testing and the solution shouldn't be "The check fails,
> remove the check".
>
> On Sat, Apr 27, 2019 at 4:18 AM, Konrad Rzeszutek Wilk
>  wrote:
> > On Fri, Apr 26, 2019 at 10:22:13PM +0530, Sumantro Mukherjee wrote:
> > >  Yup +1 from my side too. Xen is hardly tested since a lot of time.
> >
> > Hi!
> >
> > And that is thanks to one of the GRUB2 bugs that needs some love
> > from Peter Jones.
> >
> > As without that bug being fixed - it is very difficult to test it - as
> > you can't even load Xen!
> >
> > I've asked the upstream GRUB maintainer to sheed some light on the
> > confusion about multiboot2 + SecureBoot - hopefully that will resolve
> > the question.
> >
> > My vote is to have it remain as is.
> >
> > Thank you.
> > >
> > >  On Fri, Apr 26, 2019 at 10:07 PM Geoffrey Marr 
> > > wrote:
> > >
> > >  > Since F24, I haven't seen or heard of anyone who uses Xen over KVM
> > >  > anywhere other than this thread... I'm +1 for making this test an
> > >  > "Optional" one.
> > >  >
> > >  > Geoff Marr
> > >  > IRC: coremodule
> > >  >
> > >  >
> > >  > On Fri, Apr 26, 2019 at 10:33 AM Adam Williamson <
> > >  > adamw...@fedoraproject.org> wrote:
> > >  >
> > >  >> On Thu, 2017-07-06 at 13:19 -0700, Adam Williamson wrote:
> > >  >> > On Thu, 2017-07-06 at 15:59 -0400, Konrad Rzeszutek Wilk wrote:
> > >  >> > > > > I would prefer for it to remain as it is.
> > >  >> > > >
> > >  >> > > > This is only practical if it's going to be tested, and
> > > tested
> > >  >> regularly
> > >  >> > > > - not *only* on the final release candidate, right before
> > > we sign
> > >  >> off
> > >  >> > > > on the release. It needs to be tested regularly throughout
> > > the
> > >  >> release
> > >  >> > > > cycle, on the composes that are "nominated for testing".
> > >  >> > >
> > >  >> > > Right, which is why I am happy that you have pointed me to
> > > the right
> > >  >> > > place so I can be up-to-date.
> > >  >> >
> > >  >> > Great, thanks. So let's leave it as it is for now, but we'll
> > > keep an
> > >  >> > eye on this during F27 cycle. If we get to, say, Beta and
> > > there are no
> > >  >> > results for the test, that's gonna be a problem. Thanks!
> > >  >>
> > >  >> So, for Fedora 30, this was not tested throughout the whole
> > > cycle. I
> > >  >> think we can consider the proposal to remove the criterion active
> > >  >> again.
> > >  >> --
> > >  >> Adam Williamson
> > >  >> Fedora QA Community Monkey
> > >  >> IRC: adamw | Twitter: AdamW_Fedora | XMPP: adamw AT
> > > happyassassin . net
> > >  >> http://www.happyassassin.net
> > >  >> ___
> > >  >> test mailing list -- t...@lists.fedoraproject.org
> > >  >> To unsubscribe send an email to
> > > test-le...@lists.fedoraproject.org
> > >  >> Fedora Code of Conduct:
> > > https://getfedora.org/code-of-conduct.html
> > >  >> List Guidelines:
> > > https://fedoraproject.org/wiki/Mailing_list_guidelines
> > >  >> List Archives:
> > >  >> 
> > > https://lists.fedoraproject.org/archives/list/t...@lists.fedoraproject.org
> > >  >>
> > >  > ___
> > >  > test mailing list -- t...@lists.fedoraproject.org
> > >  > To unsubscribe send an email to test-le...@lists.fedoraproject.org
> > >  > Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
> > >  > List Guidelines:
> > > https://fedoraproject.org/wiki/Mailing_list_guidelines
> > >  > List Archives:
> > >  > 
> > > https://lists.fedoraproject.org/archives/list/t...@lists.fedoraproject.org
> > >  >
> > >
> > >
> > >  --
> > >  //sumantro
> > >  Fedora QE
> > >  TRIED AND PERSONALLY TESTED, ERGO TRUSTED
> > > 
> >
> > ___
> > Xen-devel mailing list
> > Xen-devel@lists.xenproject.org
> > https://lists.xenproject.org/mailman/listinfo/xen-devel

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

Re: [Xen-devel] GRUB Xen PVH chainloading

[Daniel Kiper]

On Mon, Jan 07, 2019 at 01:43:11PM +0100, Juergen Gross wrote:
> On 07/01/2019 12:41, Colin Watson wrote:
> > Hi,
> >
> > I'm working on integrating the recently-merged PVH support for GRUB into
> > the Debian packages.  As a result I find myself thinking about how to
> > handle the two-stage boot loader scheme that our packages currently
> > implement for PV.  I think that it would not be very hard to do this for
> > PVH in the manner outlined below, but my x86 asm skills aren't quite up
> > to some of the work needed in GRUB.  Assuming that nobody sees any
> > obvious holes in this, does anyone fancy giving it a go?
>
> Seems to be a very good idea.
>
> > Background
> > --
> >
> > Around the time PV support was implemented in GRUB 2, we put together a
> > scheme to minimise the coupling between the guest configuration file on
> > the host and the boot loader configuration in the guest.  The scheme and
> > its rationale are described here:
> >
> >   
> > https://wiki.xen.org/wiki/PvGrub2#Chainloading_guest_pvgrub2_from_domain_0_pvgrub2
> >
> > Essentially the same rationale applies to the PVH case: it should be
> > possible for the guest to declare its own booting arrangements (though
> > of course some hosts may wish to just provide a grub.cfg and handle all
> > that on the host side), and this should be decoupled from the GRUB image
> > provided by the host as far as possible in order to minimise
> > compatibility issues.
> >
> > There seems to be no obstacle to this in principle: just as a PV boot
> > loader can chainload another one from the guest's disk, so too could a
> > PVH boot loader chainload another one from the guest's disk.
> >
> > What needs to be done
> > -
> >
> > GRUB needs to support chainloading another PVH boot loader.  I think
> > this involves observing the existence of the XEN_ELFNOTE_PHYS32_ENTRY
> > note and following the machine state rules for the domain builder here:
> >
> >   https://xenbits.xen.org/docs/unstable-staging/misc/pvh.html
> >
> > (I had a brief go at implementing this, but foundered on my fairly
> > minimal understanding of GRUB's relocator/boot code.)
>
> The needed effort should indeed be rather small.
>
> In the moment I have no idea when I'll be able to do it, as I have
> plenty of other things to do. In case you want to try it and need some
> hints, please feel free to ask (maybe I'm able to give an answer
> without having to try to implement it myself ;-) ).
>
> > We need to define a modification to
> > https://xenbits.xen.org/docs/unstable-staging/misc/x86-xenpv-bootloader.html
> > for PVH boot loaders.  I suggest the obvious: a second-stage PVH boot
> > loader should be installed into the guest filesystem as
> > /boot/xen/pvhboot-.elf, and otherwise things generally behave the
> > same way.  I'd be happy to draft a patch to the protocol specification
> > once a proof-of-concept exists.
> >
> > The as-yet-unmerged GRUB patch to support the existing PV boot protocol
> > (https://salsa.debian.org/grub-team/grub/blob/master/debian/patches/grub-install-pvxen-paths.patch)
> > needs to be extended to support the amended protocol.  This is trivial
> > given the above.
>
> Would you mind trying to upstream this patch? I have CC-ed Daniel Kiper
> one of the grub2 maintainers), as I guess with his Xen skills he will be
> the one looking at the patch.

Yes, I am happy to help in development, e.g. give some hints if needed,
and review the patches. Sadly I am not able to do development myself
because I am busy with other stuff right now.

Daniel

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

Re: [Xen-devel] [RFC v1] kexec: Prototype for signature verification within Xen

[Daniel Kiper]

On Tue, Jan 15, 2019 at 03:15:28PM +0100, Simon Horman wrote:
> On Mon, Jan 14, 2019 at 01:52:07PM -0600, Eric DeVolder wrote:
> > These changes work in conjunction with the signature
> > verification support for Xen I published recently.
> >
> > Prior to this change, kexec supported the following
> > three modes of operation:
> >
> > kexec_load:
> > - unverified loading of kernel into Linux (original mode)
> > - unverified loading of kernel into Xen
> > kexec_file_load (the -s option to kexec):
> > - verified loading of kernel into Linux
> >
> > With the verified loading of a kernel into Linux, the scope
> > of kexec changed drastically as the kernel performs most of
> > the work that kexec previously did; the kernel does so so as
> > to reduce the risk of compromise.
> >
> > For example, the unverified loading of a kernel into Linux
> > involves locating memory within the system to load the
> > various pieces of data (kernel, initramdisk, command line)
> > as well as reserving additional memory such as the first 1MB
> > on x86 for legacy reasons as well as something known as
> > 'purgatory', a trampoline that checks the integrity of the
> > contents of loaded pieces of data, before invoking that
> > loaded kernel. The management of purgatory involves
> > manipulating an embedded ELF purgatory object file to insert
> > a memory hash value, and rewrite a few run-time switches
> > based on kexec command line parameters.
> >
> > By contrast, the verified loading essentially just passes
> > file handles for the kernel, initramdisk, and command line
> > pointer, and the kernel takes care of the rest, by
> > performing all the work that the unverified kexec load would
> > do, but inside the kernel using trusted kernel code.
> >
> > This changeset adds a fourth mode to kexec:
> >
> > - verified loading of kernel into Xen
> >
> > In general, Xen performs the signature verification on the
> > loaded kernel, much as Linux does, but that is where the
> > similarities end.  In the current Xen implementation, no
> > infrastructure is present to support reading from [Linux
> > dom0] file handles, or for manipulating ELF objects. As
> > such, without Xen support for these actions, Xen relies upon
> > kexec to provide these services, which is what this mode
> > does.
> >
> > To achieve this, this mode of operation essentially vectors
> > the verified load for Xen through the non-verified path,
> > which performs all the needed actions for kexec to work, but
> > then makes an adjustment to pass the entire kernel file, not
> > just the loadable portion of the kernel file, to Xen in
> > order to provide the proper image for signature
> > verification.
> >
> > The loading of kexec images for signature verification for
> > Xen is indicated with the -s switch, just like for Linux.
> >
> > Changes to configure.ac are for detecting whether or not the
> > Xen version supports this kexec_file_load hypercall op.
> >
> > Changes to kexec-bzImage64.c are for recording what the
> > change to the kernel image entry needs to be (the entire
> > kernel file, not just the loadable portion), as well as
> > vectoring kexec_file_load through kexec_load for Xen.
> >
> > Changes to kexec-xen.c are to invoke the new Xen
> > kexec_file_load hypercall op, from kexec_load.
> >
> > Changes to kexec.c are to vector kexec_file_load for Xen
> > throgh kexec_load for Xen, as well as make the correction
> > for passing the complete kernel file to Xen.
> >
> > Signed-off-by: Eric DeVolder 
>
> Thanks Eric,
>
> this looks good to me, aside from one nit below.

Please do not apply this patch. This is an RFC.

Eric, thank you for doing this work.

Daniel

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

Re: [Xen-devel] [PATCH v7 00/20] xen: add pvh guest support

[Daniel Kiper]

On Fri, Dec 07, 2018 at 01:11:28PM +0100, Juergen Gross wrote:
> This patch series adds support for booting Linux as PVH guest.
>
> Similar to i386/xen and x86_64/xen platforms the new i386/xenpvh
> platform grub is booted as a standalone image directly by Xen.
>
> For booting Linux kernel it is using the standard linux kernel
> loader. The only modification of the linux loader is to pass the
> ACPI RSDP address via boot parameters to the kernel, as that table
> might not be located at the usual physical address just below 1MB.
>
> The related Linux kernel patches have been accepted for 4.20-rc4
>
> Changes in V7:
> - fix build failures for various targets (patch 9)

All x86 and ARM test builds passed. If there are no further issues or
objections I will apply this patch series in the first half of next week.

Daniel

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

Re: [Xen-devel] [GRUB PATCH 1/2] verifiers: Xen fallout cleanup

[Daniel Kiper]

On Thu, Dec 06, 2018 at 10:37:43AM -0500, Ross Philipson wrote:
> On 12/06/2018 08:40 AM, Daniel Kiper wrote:
> > Xen fallout cleanup after commit ca0a4f689 (verifiers: File type for
> > fine-grained signature-verification controlling).
> >
> > Signed-off-by: Daniel Kiper 
> > ---
> >  grub-core/loader/i386/xen.c | 14 +++---
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c
> > index 1a99ca72c..8f662c8ac 100644
> > --- a/grub-core/loader/i386/xen.c
> > +++ b/grub-core/loader/i386/xen.c
> > @@ -645,10 +645,10 @@ grub_cmd_xen (grub_command_t cmd __attribute__ 
> > ((unused)),
> >
> >grub_xen_reset ();
> >
> > -  grub_create_loader_cmdline (argc - 1, argv + 1,
> > - (char *) xen_state.next_start.cmd_line,
> > - sizeof (xen_state.next_start.cmd_line) - 1);
> > -  err = grub_verify_string (xen_state.next_start.cmd_line, 
> > GRUB_VERIFY_MODULE_CMDLINE);
> > +  err = grub_create_loader_cmdline (argc - 1, argv + 1,
> > +   (char *) xen_state.next_start.cmd_line,
> > +   sizeof (xen_state.next_start.cmd_line) - 1,
> > +   GRUB_VERIFY_KERNEL_CMDLINE);
>
> How did this compile previously if you were missing an argument to
> grub_create_loader_cmdline?

This is only build if xen platform is enabled. Otherwise this file is
not used.

Daniel

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

Re: [Xen-devel] [PATCH v6 09/20] xen: add basic hooks for PVH in current code

[Daniel Kiper]

On Wed, Nov 28, 2018 at 02:55:19PM +0100, Juergen Gross wrote:
> Add the hooks to current code needed for Xen PVH. They will be filled
> with code later when the related functionality is being added.
>
> loader/i386/linux.c needs to include machine/kernel.h now as it needs
> to get GRUB_KERNEL_USE_RSDP_ADDR from there.
>
> Signed-off-by: Juergen Gross 
> Reviewed-by: Daniel Kiper 
> ---
> V3: xenpvh->xen_pvh (Daniel Kiper)
> adjust copyright date (Roger Pau Monné)
> V5: update commit message (Daniel Kiper)
> move including xen/hvm/start_info.h to the sources really needing
>   it (Daniel Kiper)
> ---

[...]

> diff --git a/grub-core/loader/i386/linux.c b/grub-core/loader/i386/linux.c
> index 375ee80dc..b6015913b 100644
> --- a/grub-core/loader/i386/linux.c
> +++ b/grub-core/loader/i386/linux.c
> @@ -35,6 +35,7 @@
>  #include 
>  #include 
>  #include 
> +#include 

This include breaks i386 ieee1275 builds. Could you fix that and repost?

Daniel

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

[Xen-devel] [GRUB PATCH 2/2] verifiers: ARM Xen fallout cleanup

[Daniel Kiper]

ARM Xen fallout cleanup after commit ca0a4f689 (verifiers: File type for
fine-grained signature-verification controlling).

Signed-off-by: Daniel Kiper 
---
 grub-core/loader/arm64/xen_boot.c | 8 
 include/grub/file.h   | 5 +
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/grub-core/loader/arm64/xen_boot.c 
b/grub-core/loader/arm64/xen_boot.c
index 33a855df4..a742868a4 100644
--- a/grub-core/loader/arm64/xen_boot.c
+++ b/grub-core/loader/arm64/xen_boot.c
@@ -429,9 +429,9 @@ grub_cmd_xen_module (grub_command_t cmd 
__attribute__((unused)),
 
   grub_dprintf ("xen_loader", "Init module and node info\n");
 
-  if (nounzip)
-grub_file_filter_disable_compression ();
-  file = grub_file_open (argv[0]);
+  file = grub_file_open (argv[0], GRUB_FILE_TYPE_XEN_MODULE
+| (nounzip ? GRUB_FILE_TYPE_NO_DECOMPRESS
+   : GRUB_FILE_TYPE_NONE));
   if (!file)
 goto fail;
 
@@ -463,7 +463,7 @@ grub_cmd_xen_hypervisor (grub_command_t cmd __attribute__ 
((unused)),
   goto fail;
 }
 
-  file = grub_file_open (argv[0]);
+  file = grub_file_open (argv[0], GRUB_FILE_TYPE_XEN_HYPERVISOR);
   if (!file)
 goto fail;
 
diff --git a/include/grub/file.h b/include/grub/file.h
index 9aae46355..cbbd29465 100644
--- a/include/grub/file.h
+++ b/include/grub/file.h
@@ -42,6 +42,11 @@ enum grub_file_type
 /* Multiboot module.  */
 GRUB_FILE_TYPE_MULTIBOOT_MODULE,
 
+/* Xen hypervisor - used on ARM only. */
+GRUB_FILE_TYPE_XEN_HYPERVISOR,
+/* Xen module - used on ARM only. */
+GRUB_FILE_TYPE_XEN_MODULE,
+
 GRUB_FILE_TYPE_BSD_KERNEL,
 GRUB_FILE_TYPE_FREEBSD_ENV,
 GRUB_FILE_TYPE_FREEBSD_MODULE,
-- 
2.11.0


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

[Xen-devel] [GRUB PATCH 1/2] verifiers: Xen fallout cleanup

[Daniel Kiper]

Xen fallout cleanup after commit ca0a4f689 (verifiers: File type for
fine-grained signature-verification controlling).

Signed-off-by: Daniel Kiper 
---
 grub-core/loader/i386/xen.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c
index 1a99ca72c..8f662c8ac 100644
--- a/grub-core/loader/i386/xen.c
+++ b/grub-core/loader/i386/xen.c
@@ -645,10 +645,10 @@ grub_cmd_xen (grub_command_t cmd __attribute__ ((unused)),
 
   grub_xen_reset ();
 
-  grub_create_loader_cmdline (argc - 1, argv + 1,
- (char *) xen_state.next_start.cmd_line,
- sizeof (xen_state.next_start.cmd_line) - 1);
-  err = grub_verify_string (xen_state.next_start.cmd_line, 
GRUB_VERIFY_MODULE_CMDLINE);
+  err = grub_create_loader_cmdline (argc - 1, argv + 1,
+   (char *) xen_state.next_start.cmd_line,
+   sizeof (xen_state.next_start.cmd_line) - 1,
+   GRUB_VERIFY_KERNEL_CMDLINE);
   if (err)
 return err;
 
@@ -910,9 +910,9 @@ grub_cmd_module (grub_command_t cmd __attribute__ 
((unused)),
   if (err)
 goto fail;
 
-  grub_create_loader_cmdline (argc - 1, argv + 1,
- get_virtual_current_address (ch), cmdline_len);
-  err = grub_verify_string (get_virtual_current_address (ch), 
GRUB_VERIFY_MODULE_CMDLINE);
+  err = grub_create_loader_cmdline (argc - 1, argv + 1,
+   get_virtual_current_address (ch), 
cmdline_len,
+   GRUB_VERIFY_MODULE_CMDLINE);
   if (err)
 goto fail;
 
-- 
2.11.0


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

[Xen-devel] [GRUB PATCH 0/2] verifiers: Fallout cleanup

[Daniel Kiper]

Hey,

Yeah, fallout cleanup after verifiers introduction. Sorry about that.

Now I test build all x86 and ARM supported platforms. I am going to
add others to my test script soon to avoid such mess in the future.

Daniel

 grub-core/loader/arm64/xen_boot.c |  8 
 grub-core/loader/i386/xen.c   | 14 +++---
 include/grub/file.h   |  5 +
 3 files changed, 16 insertions(+), 11 deletions(-)

Daniel Kiper (2):
  verifiers: Xen fallout cleanup
  verifiers: ARM Xen fallout cleanup


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

Re: [Xen-devel] Make grub error "too few arguments" with xen

[Daniel Kiper]

On Tue, Dec 04, 2018 at 05:49:24AM +, Chen, Farrah wrote:
> Hi all,
>
> When I make grub, I met error " too few arguments to function 
> 'grub_create_loader_cmdline'" with xen.
> I used git bisect and found the error occurred from commit: 
> 4d4a8c96e3593d76fe7b025665ccdecc70a53c1f.
> Do you have any ideas? Thanks a lot!
>
> commit 4d4a8c96e3593d76fe7b025665ccdecc70a53c1f
> Author: Vladimir Serbinenko 
> Date:   Tue Feb 7 02:10:14 2017 +0100
>
> verifiers: Add possibility to verify kernel and modules command lines
>
> Signed-off-by: Vladimir Serbinenko 
> Signed-off-by: Daniel Kiper 
> Reviewed-by: Ross Philipson 
>
> Make steps and Error log:
>
> cd grub
> ./autogen.sh
> ./configure --target=amd64 --with-platform=xen --prefix=${PWD}/../pvgrub2
> make
> ..
> loader/i386/xen.c: In function 'grub_cmd_xen':
> loader/i386/xen.c:650:10: error: too few arguments to function 
> 'grub_create_loader_cmdline'
>   sizeof (xen_state.next_start.cmd_line) - 1);
>   ^

[...]

Ugh... This is the fallout after verifiers framework introduction.
I will fix this and post the patches probably tomorrow. Sorry for
the confusion.

Daniel

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

Re: [Xen-devel] [PATCH v6 00/20] xen: add pvh guest support

[Daniel Kiper]

On Thu, Nov 29, 2018 at 01:40:35PM +0100, Hans van Kranenburg wrote:
> Hi Daniel,
>
> On 11/29/18 1:22 PM, Daniel Kiper wrote:
> > On Wed, Nov 28, 2018 at 02:55:10PM +0100, Juergen Gross wrote:
> >> This patch series adds support for booting Linux as PVH guest.
> >>
> >> Similar to i386/xen and x86_64/xen platforms the new i386/xenpvh
> >> platform grub is booted as a standalone image directly by Xen.
> >>
> >> For booting Linux kernel it is using the standard linux kernel
> >> loader. The only modification of the linux loader is to pass the
> >> ACPI RSDP address via boot parameters to the kernel, as that table
> >> might not be located at the usual physical address just below 1MB.
> >>
> >> The related Linux kernel patches have been accepted for 4.20-rc4
> >
> > Patch set LGTM. If there are no objections I will apply it in a week
> > or so.
> >
> > Hans, may I add "Tested-by: Hans van Kranenburg " to
> > the patches?
>
> Sure!

Great! Thanks a lot!

Daniel

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

Re: [Xen-devel] [PATCH v6 00/20] xen: add pvh guest support

[Daniel Kiper]

On Wed, Nov 28, 2018 at 02:55:10PM +0100, Juergen Gross wrote:
> This patch series adds support for booting Linux as PVH guest.
>
> Similar to i386/xen and x86_64/xen platforms the new i386/xenpvh
> platform grub is booted as a standalone image directly by Xen.
>
> For booting Linux kernel it is using the standard linux kernel
> loader. The only modification of the linux loader is to pass the
> ACPI RSDP address via boot parameters to the kernel, as that table
> might not be located at the usual physical address just below 1MB.
>
> The related Linux kernel patches have been accepted for 4.20-rc4

Patch set LGTM. If there are no objections I will apply it in a week
or so.

Hans, may I add "Tested-by: Hans van Kranenburg " to
the patches?

Juergen, thank you for doing the work.

Daniel

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

Re: [Xen-devel] [PATCH v6 16/20] grub-module-verifier: Ignore all_video for xenpvh

[Daniel Kiper]

On Wed, Nov 28, 2018 at 11:21:54PM +0100, Hans van Kranenburg wrote:
> On 11/28/18 2:55 PM, Juergen Gross wrote:
> > From: Hans van Kranenburg 
> >
> > This solves the build failing with "Error: no symbol table and no
> > .moddeps section"
> >
> > Also see:
> > - 6371e9c10433578bb236a8284ddb9ce9e201eb59
> > - https://savannah.gnu.org/bugs/?49012
> >
> > Signed-off-by: Hans van Kranenburg 
> > Reviewed-by: Daniel Kiper 
>
> Just a small detail... The xenpvh in the subject was not renamed to
> xen_pvh. That can probably be fixed up while committing. :)

Yeah, I can do that...

Daniel

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

Re: [Xen-devel] [PATCH v6 11/20] xen: setup hypercall page for PVH

[Daniel Kiper]

On Wed, Nov 28, 2018 at 02:55:21PM +0100, Juergen Gross wrote:
> Add the needed code to setup the hypercall page for calling into the
> Xen hypervisor.
>
> Import the XEN_HVM_DEBUGCONS_IOPORT define from Xen unstable into
> include/xen/arch-x86/xen.h
>
> Signed-off-by: Juergen Gross 
> Reviewed-by: Roger Pau Monné 
> ---
> V3: grub_xen_early_halt->grub_xen_panic (Roger Pau Monné)
> issue panic message (Roger Pau Monné)
> rewrite grub_xen_hypercall to avoid register variables (Daniel Kiper)
> V5: Use XEN_HVM_DEBUGCONS_IOPORT from Xen unstable (Roger Pau Monné)
> Issue "System halted!" in panic (Daniel Kiper)
>     Clear interrupts and loop for halting (Roger Pau Monné, Daniel Kiper)
> Use only one dummy variable for hypercall asm statement
> V6: Added some comments (Daniel Kiper)
> Use "+x" constraints instead of dummy variable (Daniel Kiper)
> ---
>  grub-core/kern/i386/xen/pvh.c | 80 
> +++
>  include/xen/arch-x86/xen.h|  7 
>  2 files changed, 87 insertions(+)
>
> diff --git a/grub-core/kern/i386/xen/pvh.c b/grub-core/kern/i386/xen/pvh.c
> index 4f629b15e..a2554fb1d 100644
> --- a/grub-core/kern/i386/xen/pvh.c
> +++ b/grub-core/kern/i386/xen/pvh.c
> @@ -20,15 +20,95 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  #include 
>  #include 
>  #include 
>
>  grub_uint64_t grub_rsdp_addr;
>
> +static char hypercall_page[GRUB_XEN_PAGE_SIZE]
> +  __attribute__ ((aligned (GRUB_XEN_PAGE_SIZE)));
> +
> +static grub_uint32_t xen_cpuid_base;
> +
> +static void
> +grub_xen_cons_msg (const char *msg)
> +{
> +  const char *c;
> +
> +  for (c = msg; *c; c++)
> +grub_outb (*c, XEN_HVM_DEBUGCONS_IOPORT);
> +}
> +
> +static void
> +grub_xen_panic (const char *msg)
> +{
> +  grub_xen_cons_msg (msg);
> +  grub_xen_cons_msg ("System halted!\n");
> +
> +  asm volatile ("cli");
> +
> +  while (1)
> +{
> +  asm volatile ("hlt");
> +}
> +}
> +
> +static void
> +grub_xen_cpuid_base (void)
> +{
> +  grub_uint32_t base, eax, signature[3];
> +
> +  for (base = 0x4000; base < 0x4001; base += 0x100)
> +{
> +  grub_cpuid (base, eax, signature[0], signature[1], signature[2]);
> +  if (!grub_memcmp ("XenVMMXenVMM", signature, 12) && (eax - base) >= 2)
> + {
> +   xen_cpuid_base = base;
> +   return;
> + }
> +}
> +
> +  grub_xen_panic ("Found no Xen signature!\n");
> +}
> +
> +static void
> +grub_xen_setup_hypercall_page (void)
> +{
> +  grub_uint32_t msr, addr, eax, ebx, ecx, edx;
> +
> +  /* Get base address of Xen-specific MSRs. */
> +  grub_cpuid (xen_cpuid_base + 2, eax, ebx, ecx, edx);
> +  msr = ebx;
> +  addr = (grub_uint32_t) (_page);
> +
> +  /* Specify hypercall page address for Xen. */
> +  asm volatile ("wrmsr" : : "c" (msr), "a" (addr), "d" (0) : "memory");
> +}
> +
> +int
> +grub_xen_hypercall (grub_uint32_t callno, grub_uint32_t a0,
> + grub_uint32_t a1, grub_uint32_t a2,
> + grub_uint32_t a3, grub_uint32_t a4,
> + grub_uint32_t a5 __attribute__ ((unused)))
> +{
> +  grub_uint32_t res;
> +
> +  asm volatile ("call *%[callno]"
> + : "=a" (res), "+b" (a0), "+c" (a1), "+d" (a2),
> +   "+S" (a3), "+D" (a4)
> + : [callno] "a" (_page[callno * 32])

I am not sure why you so tightly tie compiler hands using "a" constraint for
[callno]. IMO you can use "rm" here. However, this not change anything in
the output code, so, Reviewed-by: Daniel Kiper 

Daniel

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

Re: [Xen-devel] [PATCH v5 01/20] xen: add some xen headers

[Daniel Kiper]

On Wed, Nov 21, 2018 at 03:28:36PM +0100, Juergen Gross wrote:
> In order to support grub2 in Xen PVH environment some additional Xen
> headers are needed as grub2 will be started in PVH mode requiring to
> use several HVM hypercalls and structures.
>
> Add the needed headers from Xen 4.10 being the first Xen version with
> full (not only experimental) PVH guest support.
>
> Signed-off-by: Juergen Gross 
> Reviewed-by: Daniel Kiper 

When I apply this patch I get this:

  Applying: xen: add some xen headers
  .git/rebase-apply/patch:723: trailing whitespace.
   *
  .git/rebase-apply/patch:725: trailing whitespace.
   *
  .git/rebase-apply/patch:764: trailing whitespace.
   * Maximum # bits addressable by the user of the allocated region (e.g., I/O
  .git/rebase-apply/patch:765: trailing whitespace.
   * devices often have a 32-bit limitation even in 64-bit systems). If zero
  .git/rebase-apply/patch:766: trailing whitespace.
   * then the user has no addressing restriction. This field is not used by
  warning: squelched 7 whitespace errors
  warning: 12 lines add whitespace errors.

Could you fix this?

Daniel

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

Re: [Xen-devel] [PATCH v5 11/20] xen: setup hypercall page for PVH

[Daniel Kiper]

On Tue, Nov 27, 2018 at 09:31:10PM +0100, Daniel Kiper wrote:
> On Wed, Nov 21, 2018 at 03:28:46PM +0100, Juergen Gross wrote:
> > Add the needed code to setup the hypercall page for calling into the
> > Xen hypervisor.
> >
> > Import the XEN_HVM_DEBUGCONS_IOPORT define from Xen unstable into
> > include/xen/arch-x86/xen.h
> >
> > Signed-off-by: Juergen Gross 

[...]

> > +int
> > +grub_xen_hypercall (grub_uint32_t callno, grub_uint32_t a0,
> > +   grub_uint32_t a1, grub_uint32_t a2,
> > +   grub_uint32_t a3, grub_uint32_t a4,
> > +   grub_uint32_t a5 __attribute__ ((unused)))
> > +{
> > +  grub_uint32_t __res, dummy;
> > +
> > +  asm volatile ("call *%[callno]"
> > +   : "=a" (__res), "=b" (dummy), "=c" (dummy), "=d" (dummy),
> > + "=S" (dummy), "=D" (dummy)
> > +   : "1" (a0), "2" (a1), "3" (a2), "4" (a3), "5" (a4),
> > + [callno] "a" (_page[callno * 32])
> > +   : "memory");
>
> Have you tried "+b", "+c", ... instead of "=b", "=c", ...?

I have done some experiments. Your code:

  grub_uint32_t __res, dummy;

  asm volatile ("call *%[callno]"
: "=a" (__res), "=b" (dummy), "=c" (dummy), "=d" (dummy),
  "=S" (dummy), "=D" (dummy)
: "1" (a0), "2" (a1), "3" (a2), "4" (a3), "5" (a4),
  [callno] "a" (_page[callno * 32])
: "memory");

... generates this assembly:

048e :
 48e:   55  push   %ebp
 48f:   89 e5   mov%esp,%ebp
 491:   57  push   %edi
 492:   56  push   %esi
 493:   53  push   %ebx
 494:   c1 e0 05shl$0x5,%eax
 497:   05 00 10 00 00  add$0x1000,%eax
 49c:   89 d3   mov%edx,%ebx
 49e:   8b 55 08mov0x8(%ebp),%edx
 4a1:   8b 75 0cmov0xc(%ebp),%esi
 4a4:   8b 7d 10mov0x10(%ebp),%edi
 4a7:   ff d0   call   *%eax
 4a9:   5b  pop%ebx
 4aa:   5e  pop%esi
 4ab:   5f  pop%edi
 4ac:   5d  pop%ebp
 4ad:   c2 10 00ret$0x10

Mine code:

  grub_uint32_t __res;

  asm volatile ("call *%[callno]"
: "=a" (__res), "+b" (a0), "+c" (a1), "+d" (a2), "+S" (a3), 
"+D" (a4)
: [callno] "rm" (_page[callno * 32])
: "memory");

... generates this assembly:

048e :
 48e:   55  push   %ebp
 48f:   89 e5   mov%esp,%ebp
 491:   57  push   %edi
 492:   56  push   %esi
 493:   53  push   %ebx
 494:   c1 e0 05shl$0x5,%eax
 497:   05 00 10 00 00  add$0x1000,%eax
 49c:   89 d3   mov%edx,%ebx
 49e:   8b 55 08mov0x8(%ebp),%edx
 4a1:   8b 75 0cmov0xc(%ebp),%esi
 4a4:   8b 7d 10mov0x10(%ebp),%edi
 4a7:   ff d0   call   *%eax
 4a9:   5b  pop%ebx
 4aa:   5e  pop%esi
 4ab:   5f  pop%edi
 4ac:   5d  pop%ebp
 4ad:   c2 10 00ret$0x10

So, both are equal but mine seems a bit simpler.

And I think that you can drop "__" from __res variable.

Daniel

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

Re: [Xen-devel] [PATCH v5 15/20] xen_pvh: add build runes for grub-core

[Daniel Kiper]

On Wed, Nov 21, 2018 at 03:28:50PM +0100, Juergen Gross wrote:
> Add the modifications to the build system needed to build a xen_pvh
> grub.
>
> Signed-off-by: Juergen Gross 
> Reviewed-by: Daniel Kiper 
> ---
> V3: sorted some filenames (Daniel Kiper)
> V4: add bus/pci.c to xen_pvh

V5 drops bus/pci.c from xen_pvh. Is it intentional or mistake?

Daniel

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

Re: [Xen-devel] [PATCH v5 13/20] xen: setup Xen specific data for PVH

[Daniel Kiper]

On Wed, Nov 21, 2018 at 03:28:48PM +0100, Juergen Gross wrote:
> Initialize the needed Xen specific data. This is:
>
> - the Xen start of day page containing the console and Xenstore ring
>   page PFN and event channel
> - the grant table
> - the shared info page
>
> Write back the possibly modified memory map to the hypervisor in case
> the guest is reading it from there again.
>
> Set the RSDP address for the guest from the start_info page passed
> as boot parameter.
>
> Signed-off-by: Juergen Gross 
> Reviewed-by: Daniel Kiper 

One nitpick below...

> ---
> V4: write back memory map to Xen (Roger Pau Monné)
> V5: add comment (Daniel Kiper)
> ---
>  grub-core/kern/i386/xen/pvh.c | 120 
> ++
>  1 file changed, 120 insertions(+)
>
> diff --git a/grub-core/kern/i386/xen/pvh.c b/grub-core/kern/i386/xen/pvh.c
> index bb90874b3..6de84eb8e 100644
> --- a/grub-core/kern/i386/xen/pvh.c
> +++ b/grub-core/kern/i386/xen/pvh.c
> @@ -27,6 +27,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>
>  #define XEN_MEMORY_MAP_SIZE   128
> @@ -37,6 +38,7 @@ static char hypercall_page[GRUB_XEN_PAGE_SIZE]
>__attribute__ ((aligned (GRUB_XEN_PAGE_SIZE)));
>
>  static grub_uint32_t xen_cpuid_base;
> +static struct start_info grub_xen_start_page;
>  static struct grub_e820_mmap_entry map[XEN_MEMORY_MAP_SIZE];
>  static unsigned int nr_map_entries;
>
> @@ -110,6 +112,36 @@ grub_xen_hypercall (grub_uint32_t callno, grub_uint32_t 
> a0,
>return __res;
>  }
>
> +static grub_uint32_t
> +grub_xen_get_param (int idx)
> +{
> +  struct xen_hvm_param xhv;
> +  int r;
> +
> +  xhv.domid = DOMID_SELF;
> +  xhv.index = idx;
> +  r = grub_xen_hypercall (__HYPERVISOR_hvm_op, HVMOP_get_param,
> +   (grub_uint32_t) (), 0, 0, 0, 0);
> +  if (r < 0)
> +grub_xen_panic ("Could not get parameter from Xen!\n");
> +  return xhv.value;
> +}
> +
> +static void *
> +grub_xen_add_physmap (unsigned int space, void *addr)
> +{
> +  struct xen_add_to_physmap xatp;
> +
> +  xatp.domid = DOMID_SELF;
> +  xatp.idx = 0;
> +  xatp.space = space;
> +  xatp.gpfn = (grub_addr_t) addr >> GRUB_XEN_LOG_PAGE_SIZE;
> +  if (grub_xen_hypercall (__HYPERVISOR_memory_op, XENMEM_add_to_physmap,
> +   (grub_uint32_t) (), 0, 0, 0, 0))
> +grub_xen_panic ("Memory_op hypercall failed!\n");
> +  return addr;
> +}
> +
>  static void
>  grub_xen_sort_mmap (void)
>  {
> @@ -196,12 +228,100 @@ grub_xen_get_mmap (void)
>grub_xen_sort_mmap ();
>  }
>
> +static void
> +grub_xen_set_mmap (void)
> +{
> +  struct xen_foreign_memory_map memmap;
> +
> +  memmap.domid = DOMID_SELF;
> +  memmap.map.nr_entries = nr_map_entries;
> +  set_xen_guest_handle (memmap.map.buffer, map);
> +  grub_xen_hypercall (__HYPERVISOR_memory_op, XENMEM_set_memory_map,
> +   (grub_uint32_t) (), 0, 0, 0, 0);
> +}
> +
> +static grub_uint64_t
> +grub_xen_find_page (grub_uint64_t start)
> +{
> +  unsigned int i, j;
> +  grub_uint64_t last = start;
> +
> +  /* Try to find a e820 map hole below 4G. */
> +  /* Relies on page-aligned entries (addr and len) and input (start). */

I would like to see above two comments as one as below.

/*
 * Try to find a e820 map hole below 4G.
 * Relies on page-aligned entries (addr and len) and input (start).
 */

You can retain my RB if you change that.

Daniel

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

Re: [Xen-devel] [PATCH v5 11/20] xen: setup hypercall page for PVH

[Daniel Kiper]

On Wed, Nov 21, 2018 at 03:28:46PM +0100, Juergen Gross wrote:
> Add the needed code to setup the hypercall page for calling into the
> Xen hypervisor.
>
> Import the XEN_HVM_DEBUGCONS_IOPORT define from Xen unstable into
> include/xen/arch-x86/xen.h
>
> Signed-off-by: Juergen Gross 
> ---
> V3: grub_xen_early_halt->grub_xen_panic (Roger Pau Monné)
> issue panic message (Roger Pau Monné)
> rewrite grub_xen_hypercall to avoid register variables (Daniel Kiper)
> V5: Use XEN_HVM_DEBUGCONS_IOPORT from Xen unstable (Roger Pau Monné)
>     Issue "System halted!" in panic (Daniel Kiper)
> Clear interrupts and loop for halting (Roger Pau Monné, Daniel Kiper)
> Use only one dummy variable for hypercall asm statement
> ---
>  grub-core/kern/i386/xen/pvh.c | 79 
> +++
>  include/xen/arch-x86/xen.h|  7 
>  2 files changed, 86 insertions(+)
>
> diff --git a/grub-core/kern/i386/xen/pvh.c b/grub-core/kern/i386/xen/pvh.c
> index 4f629b15e..478cef0d1 100644
> --- a/grub-core/kern/i386/xen/pvh.c
> +++ b/grub-core/kern/i386/xen/pvh.c
> @@ -20,15 +20,94 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  #include 
>  #include 
>  #include 
>
>  grub_uint64_t grub_rsdp_addr;
>
> +static char hypercall_page[GRUB_XEN_PAGE_SIZE]
> +  __attribute__ ((aligned (GRUB_XEN_PAGE_SIZE)));
> +
> +static grub_uint32_t xen_cpuid_base;
> +
> +static void
> +grub_xen_cons_msg (const char *msg)
> +{
> +  const char *c;
> +
> +  for (c = msg; *c; c++)
> +grub_outb (*c, XEN_HVM_DEBUGCONS_IOPORT);
> +}
> +
> +static void
> +grub_xen_panic (const char *msg)
> +{
> +  grub_xen_cons_msg (msg);
> +  grub_xen_cons_msg ("System halted!\n");
> +
> +  asm volatile ("cli");
> +
> +  while (1)
> +{
> +  asm volatile ("hlt");
> +}
> +}
> +
> +static void
> +grub_xen_cpuid_base (void)
> +{
> +  grub_uint32_t base, eax, signature[3];
> +
> +  for (base = 0x4000; base < 0x4001; base += 0x100)
> +{
> +  grub_cpuid (base, eax, signature[0], signature[1], signature[2]);
> +  if (!grub_memcmp ("XenVMMXenVMM", signature, 12) && (eax - base) >= 2)
> + {
> +   xen_cpuid_base = base;
> +   return;
> + }
> +}
> +
> +  grub_xen_panic ("Found no Xen signature!\n");
> +}
> +
> +static void
> +grub_xen_setup_hypercall_page (void)
> +{
> +  grub_uint32_t msr, pfn, eax, ebx, ecx, edx;
> +
> +  grub_cpuid (xen_cpuid_base + 2, eax, ebx, ecx, edx);

Could not you use a constant instead of plain 2 here? Is there anything
like that in Xen headers? If not please add a comment what grub_cpuid ()
and wrmsr do. One line is sufficient.

> +  msr = ebx;
> +  pfn = (grub_uint32_t) (_page);

Is it PFN? Really? I do not think so.

> +
> +  asm volatile ("wrmsr" : : "c" (msr), "a" (pfn), "d" (0) : "memory");
> +}
> +
> +int
> +grub_xen_hypercall (grub_uint32_t callno, grub_uint32_t a0,
> + grub_uint32_t a1, grub_uint32_t a2,
> + grub_uint32_t a3, grub_uint32_t a4,
> + grub_uint32_t a5 __attribute__ ((unused)))
> +{
> +  grub_uint32_t __res, dummy;
> +
> +  asm volatile ("call *%[callno]"
> + : "=a" (__res), "=b" (dummy), "=c" (dummy), "=d" (dummy),
> +   "=S" (dummy), "=D" (dummy)
> + : "1" (a0), "2" (a1), "3" (a2), "4" (a3), "5" (a4),
> +   [callno] "a" (_page[callno * 32])
> + : "memory");

Have you tried "+b", "+c", ... instead of "=b", "=c", ...?

Daniel

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

Re: [Xen-devel] [PATCH v5 09/20] xen: add basic hooks for PVH in current code

[Daniel Kiper]

On Wed, Nov 21, 2018 at 03:28:44PM +0100, Juergen Gross wrote:
> Add the hooks to current code needed for Xen PVH. They will be filled
> with code later when the related functionality is being added.
>
> loader/i386/linux.c needs to include machine/kernel.h now as it needs
> to get GRUB_KERNEL_USE_RSDP_ADDR from there.
>
> Signed-off-by: Juergen Gross 

Reviewed-by: Daniel Kiper 

Daniel

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

Re: [Xen-devel] [PATCH v5 07/20] xen: modify grub_xen_ptr2mfn() for xen-pvh

[Daniel Kiper]

On Wed, Nov 21, 2018 at 03:28:42PM +0100, Juergen Gross wrote:
> grub_xen_ptr2mfn() returns the machine frame number for a given pointer
> value. For Xen-PVH guests this is just the PFN. Add the PVH specific
> variant.
>
> Signed-off-by: Juergen Gross 

Reviewed-by: Daniel Kiper 

Daniel

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

Re: [Xen-devel] [PATCH v5 05/20] xen: add some dummy headers for PVH mode

[Daniel Kiper]

On Wed, Nov 21, 2018 at 03:28:40PM +0100, Juergen Gross wrote:
> With Xen PVH mode adding a new machine type the machine related headers
> need to be present for the build to succeed. Most of the headers just
> need to include the related common i386 headers. Add those to the tree.
>
> Note that xen_pvh/int.h needs to include pc/int_types.h instead of
> pc/int.h in order to avoid the definition of grub_bios_interrupt().
>
> xen_pvh/memory.h needs to include coreboot/memory.h (like some other
> /memory.h do as well) as this contains just the needed stubs.
>
> Signed-off-by: Juergen Gross 

Reviewed-by: Daniel Kiper 

Daniel

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

Re: [Xen-devel] [PATCH v5 02/20] loader/linux: support passing rsdp address via boot params

[Daniel Kiper]

On Wed, Nov 21, 2018 at 03:28:37PM +0100, Juergen Gross wrote:
> Xen PVH guests will have the RSDP at an arbitrary address. Support that
> by passing the RSDP address via the boot parameters to Linux.
>
> Signed-off-by: Juergen Gross 

Reviewed-by: Daniel Kiper 

Daniel

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

Re: [Xen-devel] [PATCH v4 12/19] xen: add PCI MMIO areas to memory map

[Daniel Kiper]

On Wed, Nov 14, 2018 at 01:49:16PM +0100, Roger Pau Monné wrote:
> On Fri, Nov 09, 2018 at 08:14:57PM +0100, Daniel Kiper wrote:
> > On Fri, Nov 02, 2018 at 01:37:31PM +0100, Juergen Gross wrote:
> > > Add possible PCI space MMIO areas as "Reserved" to the memory map in
> > > order to avoid using those areas for special Xen pages later.
> > >
> > > Signed-off-by: Juergen Gross 
> >
> > Reviewed-by: Daniel Kiper  but I would like to
> > here something from Roger here too.
>
> I've almost missed this one, could you please Cc me next time?

Ahhh, sorry about that. However, I think that it will be much easier if
Juergen will CC you on next version of these patches. Juergen?

Daniel

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

Re: [Xen-devel] [PATCH v4 17/19] xen_pvh: support building a standalone image

[Daniel Kiper]

On Fri, Nov 02, 2018 at 01:37:36PM +0100, Juergen Gross wrote:
> Support mkimage for xen_pvh.
>
> In order to avoid using plain integers for the ELF notes use the
> available Xen include instead. While at it replace the plain numbers
> for Xen PV mode, too.
>
> Signed-off-by: Juergen Gross 
> Reviewed-by: Daniel Kiper 
> ---
> V2: some style adjustments (Daniel Kiper)
> use defines for elf-notes (Daniel Kiper)

Thanks a lot! However, I would like to ask you to move the
latter to separate patch. You can retain my RB though.

Daniel

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

Re: [Xen-devel] [PATCH v4 14/19] xen: init memory regions for PVH

[Daniel Kiper]

On Fri, Nov 02, 2018 at 01:37:33PM +0100, Juergen Gross wrote:
> Add all usable memory regions to grub memory management and add the
> needed mmap iterate code, which will be used by grub core (e.g.
> grub-core/lib/relocator.c or grub-core/mmap/mmap.c).
>
> As we are running in 32-bit mode don't add memory above 4GB.
>
> Signed-off-by: Juergen Gross 

Reviewed-by: Daniel Kiper 

Daniel

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

Re: [Xen-devel] [PATCH v4 13/19] xen: setup Xen specific data for PVH

[Daniel Kiper]

On Fri, Nov 02, 2018 at 01:37:32PM +0100, Juergen Gross wrote:
> Initialize the needed Xen specific data. This is:
>
> - the Xen start of day page containing the console and Xenstore ring
>   page PFN and event channel
> - the grant table
> - the shared info page
>
> Write back the possibly modified memory map to the hypervisor in case
> the guest is reading it from there again.
>
> Set the RSDP address for the guest from the start_info page passed
> as boot parameter.
>
> Signed-off-by: Juergen Gross 
> ---
> V4: write back memory map to Xen (Roger Pau Monné)
> ---
>  grub-core/kern/i386/xen/pvh.c | 119 
> ++
>  1 file changed, 119 insertions(+)
>
> diff --git a/grub-core/kern/i386/xen/pvh.c b/grub-core/kern/i386/xen/pvh.c
> index 442351d1d..d74301f92 100644
> --- a/grub-core/kern/i386/xen/pvh.c
> +++ b/grub-core/kern/i386/xen/pvh.c
> @@ -27,6 +27,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>
>  #define XEN_CONSOLE_PORT  0xe9
> @@ -39,6 +40,7 @@ static char hypercall_page[GRUB_XEN_PAGE_SIZE]
>__attribute__ ((aligned (GRUB_XEN_PAGE_SIZE)));
>
>  static grub_uint32_t xen_cpuid_base;
> +static struct start_info grub_xen_start_page;
>  static struct grub_e820_mmap_entry map[XEN_MEMORY_MAP_SIZE];
>  static unsigned int nr_map_entries;
>
> @@ -100,6 +102,36 @@ grub_xen_hypercall (grub_uint32_t callno, grub_uint32_t 
> a0,
>return __res;
>  }
>
> +static grub_uint32_t
> +grub_xen_get_param (int idx)
> +{
> +  struct xen_hvm_param xhv;
> +  int r;
> +
> +  xhv.domid = DOMID_SELF;
> +  xhv.index = idx;
> +  r = grub_xen_hypercall (__HYPERVISOR_hvm_op, HVMOP_get_param,
> +   (grub_uint32_t) (), 0, 0, 0, 0);
> +  if (r < 0)
> +grub_xen_panic ("Could not get parameter from Xen.\n");

I would replace "." with "!" here and in the other grub_xen_panic() calls.

> +  return xhv.value;
> +}
> +
> +static void *
> +grub_xen_add_physmap (unsigned int space, void *addr)
> +{
> +  struct xen_add_to_physmap xatp;
> +
> +  xatp.domid = DOMID_SELF;
> +  xatp.idx = 0;
> +  xatp.space = space;
> +  xatp.gpfn = (grub_addr_t) addr >> GRUB_XEN_LOG_PAGE_SIZE;
> +  if (grub_xen_hypercall (__HYPERVISOR_memory_op, XENMEM_add_to_physmap,
> +   (grub_uint32_t) (), 0, 0, 0, 0))
> +grub_xen_panic ("Memory_op hypercall failed.\n");
> +  return addr;
> +}
> +
>  static void
>  grub_xen_sort_mmap (void)
>  {
> @@ -255,12 +287,99 @@ grub_xen_get_mmap (void)
>grub_xen_sort_mmap ();
>  }
>
> +static void
> +grub_xen_set_mmap (void)
> +{
> +  struct xen_foreign_memory_map memmap;
> +
> +  memmap.domid = DOMID_SELF;
> +  memmap.map.nr_entries = nr_map_entries;
> +  set_xen_guest_handle (memmap.map.buffer, map);
> +  grub_xen_hypercall (__HYPERVISOR_memory_op, XENMEM_set_memory_map,
> +   (grub_uint32_t) (), 0, 0, 0, 0);
> +}
> +
> +static grub_uint64_t
> +grub_xen_find_page (grub_uint64_t start)
> +{
> +  unsigned int i, j;
> +  grub_uint64_t last = start;
> +
> +  /* Try to find a e820 map hole below 4G. */
> +  for (i = 0; i < nr_map_entries; i++)
> +{
> +  if (last > map[i].addr + map[i].len)
> + continue;
> +  if (last < map[i].addr)
> + return last;
> +  if ((map[i].addr >> 32) || ((map[i].addr + map[i].len) >> 32))
> + break;
> +  last = map[i].addr + map[i].len;
> +}

It took me some time to get why it works. AIUI this is due to proper
start/end alignment. Right? So, please say about that in the comment.

And I would like to hear a word from Roger here too. If he is OK
with the patch you can add my RB too.

Daniel

> +if (i == nr_map_entries)
> +  return last;
> +
> +  /* No hole found, use the highest RAM page below 4G and reserve it. */
> +  if (nr_map_entries == ARRAY_SIZE (map))
> +grub_xen_panic ("Memory map size limit reached.\n");
> +  for (i = 0, j = 0; i < nr_map_entries; i++)
> +{
> +  if (map[i].type != GRUB_MEMORY_AVAILABLE)
> + continue;
> +  if (map[i].addr >> 32)
> + break;
> +  j = i;
> +  if ((map[i].addr + map[i].len) >> 32)
> + break;
> +}
> +  if (map[j].type != GRUB_MEMORY_AVAILABLE)
> +grub_xen_panic ("No free memory page found.\n");
> +  if ((map[j].addr + map[j].len) >> 32)
> +last = (1ULL << 32) - GRUB_XEN_PAGE_SIZE;
> +  else
> +last = map[j].addr + map[j].len - GRUB_XEN_PAGE_SIZE;
> +  map[nr_map_entries].addr = last;
> +  map[nr_map_entries].len = GRUB_XEN_PAGE_SIZE;
> +  map[nr_map_entries].type = GRUB_MEMORY_RESERVED;
> +  nr_map_entries++;
> +  grub_xen_sort_mmap ();
> +
> +  return last;
> +}
> +
>  void
>  grub_xen_setup_pvh (void)
>  {
> +  grub_addr_t par;
> +
>grub_xen_cpuid_base ();
>grub_xen_setup_hypercall_page ();
>grub_xen_get_mmap ();
> +
> +  /* Setup Xen data. */
> +  grub_xen_start_page_addr = _xen_start_page;
> +
> +  par = grub_xen_get_param (HVM_PARAM_CONSOLE_PFN);
> +  grub_xen_start_page_addr->console.domU.mfn = par;
> +  grub_xen_xcons = 

Re: [Xen-devel] [PATCH v4 12/19] xen: add PCI MMIO areas to memory map

[Daniel Kiper]

On Fri, Nov 02, 2018 at 01:37:31PM +0100, Juergen Gross wrote:
> Add possible PCI space MMIO areas as "Reserved" to the memory map in
> order to avoid using those areas for special Xen pages later.
>
> Signed-off-by: Juergen Gross 

Reviewed-by: Daniel Kiper  but I would like to
here something from Roger here too.

Daniel

> ---
> V4: new patch (Roger Pau Monné)
> ---
>  grub-core/kern/i386/xen/pvh.c | 70 
> +++
>  1 file changed, 70 insertions(+)
>
> diff --git a/grub-core/kern/i386/xen/pvh.c b/grub-core/kern/i386/xen/pvh.c
> index 58e6fefd5..442351d1d 100644
> --- a/grub-core/kern/i386/xen/pvh.c
> +++ b/grub-core/kern/i386/xen/pvh.c
> @@ -20,6 +20,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -170,6 +171,73 @@ grub_xen_sort_mmap (void)
>  }
>  }
>
> +static grub_uint64_t
> +grub_xen_pci_read (grub_pci_address_t addr, grub_uint32_t is_64bit)
> +{
> +  grub_uint64_t val;
> +
> +  val = grub_pci_read (addr);
> +  if (is_64bit)
> +{
> +  addr += sizeof (grub_uint32_t);
> +  val |= ((grub_uint64_t) grub_pci_read (addr)) << 32;
> +}
> +
> +  return val;
> +}
> +
> +static void
> +grub_xen_pci_write (grub_pci_address_t addr, grub_uint64_t val,
> + grub_uint32_t is_64bit)
> +{
> +  grub_pci_write (addr, (grub_uint32_t) val);
> +  if (is_64bit)
> +{
> +  addr += sizeof (grub_uint32_t);
> +  grub_pci_write (addr, val >> 32);
> +}
> +}
> +
> +static int
> +grub_xen_pci_mmap (grub_pci_device_t dev,
> +grub_pci_id_t pciid __attribute__ ((unused)),
> +void *data __attribute__ ((unused)))
> +{
> +  int reg;
> +  grub_pci_address_t addr;
> +  grub_uint32_t val;
> +  grub_uint64_t mmio_addr, mmio_size;
> +
> +  if (nr_map_entries == ARRAY_SIZE (map))
> +return 1;
> +
> +  for (reg = GRUB_PCI_REG_ADDRESSES; reg < GRUB_PCI_REG_CIS_POINTER;
> +   reg += sizeof (grub_uint32_t))
> +{
> +  addr = grub_pci_make_address (dev, reg);
> +  val = grub_pci_read (addr);
> +  if (val == 0 ||
> +   (val & GRUB_PCI_ADDR_SPACE_MASK) != GRUB_PCI_ADDR_SPACE_MEMORY)
> + continue;
> +
> +  val &= GRUB_PCI_ADDR_MEM_TYPE_MASK;
> +  mmio_addr = grub_xen_pci_read (addr, val);
> +  grub_xen_pci_write (addr, ~0ULL, val);
> +  mmio_size = ~(grub_xen_pci_read (addr, val) & ~0x0fULL) + 1;
> +  grub_xen_pci_write (addr, mmio_addr, val);
> +
> +  map[nr_map_entries].type = GRUB_MEMORY_RESERVED;
> +  map[nr_map_entries].addr = mmio_addr;
> +  map[nr_map_entries].len = mmio_size;
> +  nr_map_entries++;
> +
> +  if (val)
> + reg += sizeof (grub_uint32_t);
> +}
> +
> +  return 0;
> +}
> +
>  static void
>  grub_xen_get_mmap (void)
>  {
> @@ -182,6 +250,8 @@ grub_xen_get_mmap (void)
>  grub_xen_panic ("Could not get memory map from Xen.\n");
>nr_map_entries = memmap.nr_entries;
>
> +  grub_pci_iterate (grub_xen_pci_mmap, NULL);
> +
>grub_xen_sort_mmap ();
>  }
>
> --
> 2.16.4

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

Re: [Xen-devel] [PATCH v4 11/19] xen: get memory map from hypervisor for PVH

[Daniel Kiper]

On Fri, Nov 02, 2018 at 01:37:30PM +0100, Juergen Gross wrote:
> Retrieve the memory map from the hypervisor and normalize it to contain
> no overlapping entries and to be sorted by address.
>
> Signed-off-by: Juergen Gross 

One nit pick below. Otherwise
  Reviewed-by: Daniel Kiper 

> ---
> V3: use grub_e820_mmap_entry instead of own struct (Daniel Kiper)
> ---
>  grub-core/kern/i386/xen/pvh.c | 96 
> ++-
>  1 file changed, 95 insertions(+), 1 deletion(-)
>
> diff --git a/grub-core/kern/i386/xen/pvh.c b/grub-core/kern/i386/xen/pvh.c
> index 7e90a4538..58e6fefd5 100644
> --- a/grub-core/kern/i386/xen/pvh.c
> +++ b/grub-core/kern/i386/xen/pvh.c
> @@ -23,9 +23,14 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
> +#include 
> +#include 
>
> -#define XEN_CONSOLE_PORT   0xe9
> +#define XEN_CONSOLE_PORT  0xe9

Probably this will disappear but if not please put properly aligned
values in earlier patch to avoid such code shuffling.

Daniel

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

Re: [Xen-devel] [PATCH v4 10/19] xen: setup hypercall page for PVH

[Daniel Kiper]

On Fri, Nov 02, 2018 at 01:37:29PM +0100, Juergen Gross wrote:
> Add the needed code to setup the hypercall page for calling into the
> Xen hypervisor.
>
> Signed-off-by: Juergen Gross 
> ---
> V3: grub_xen_early_halt->grub_xen_panic (Roger Pau Monné)
> issue panic message (Roger Pau Monné)
> rewrite grub_xen_hypercall to avoid register variables (Daniel Kiper)
> ---
>  grub-core/kern/i386/xen/pvh.c | 69 
> +++
>  1 file changed, 69 insertions(+)
>
> diff --git a/grub-core/kern/i386/xen/pvh.c b/grub-core/kern/i386/xen/pvh.c
> index ac6181f4e..7e90a4538 100644
> --- a/grub-core/kern/i386/xen/pvh.c
> +++ b/grub-core/kern/i386/xen/pvh.c
> @@ -20,14 +20,83 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  #include 
>  #include 
>
> +#define XEN_CONSOLE_PORT   0xe9

I think that this is not PVH specific thing. Could you move this to
more generic Xen header?

>  grub_uint64_t grub_rsdp_addr;

Hmmm... It seems to me that immediately after patch #8 GRUB2 build is
broken. Is not it?

> +static char hypercall_page[GRUB_XEN_PAGE_SIZE]
> +  __attribute__ ((aligned (GRUB_XEN_PAGE_SIZE)));
> +
> +static grub_uint32_t xen_cpuid_base;
> +
> +static void
> +grub_xen_panic (const char *msg)
> +{
> +  const char *c;
> +
> +  for (c = msg; *c; c++)
> +grub_outb (*c, XEN_CONSOLE_PORT);
> +
> +  asm volatile ("hlt");

Should not you do something similar to grub-core/lib/i386/halt.c:stop() here?

> +}
> +
> +static void
> +grub_xen_cpuid_base (void)
> +{
> +  grub_uint32_t base, eax, signature[3];
> +
> +  for (base = 0x4000; base < 0x4001; base += 0x100)
> +{
> +  grub_cpuid (base, eax, signature[0], signature[1], signature[2]);
> +  if (!grub_memcmp ("XenVMMXenVMM", signature, 12) && (eax - base) >= 2)
> + {
> +   xen_cpuid_base = base;
> +   return;
> + }
> +}
> +
> +  grub_xen_panic ("Found no Xen signature.\n");

"Found no Xen signature!\nSystem halted!\n"

Or maybe grub_xen_panic() should always add "System halted!\n".

> +}
> +
> +static void
> +grub_xen_setup_hypercall_page (void)
> +{
> +  grub_uint32_t msr, pfn, eax, ebx, ecx, edx;
> +
> +  grub_cpuid (xen_cpuid_base + 2, eax, ebx, ecx, edx);
> +  msr = ebx;
> +  pfn = (grub_uint32_t) (_page[0]);

Could not you use hypercall_page alone here?

> +
> +  asm volatile ("wrmsr" : : "c" (msr), "a" (pfn), "d" (0) : "memory");
> +}
> +
> +int
> +grub_xen_hypercall (grub_uint32_t callno, grub_uint32_t a0,
> + grub_uint32_t a1, grub_uint32_t a2,
> + grub_uint32_t a3, grub_uint32_t a4,
> + grub_uint32_t a5 __attribute__ ((unused)))
> +{
> +  grub_uint32_t __res, __ign0, __ign1, __ign2, __ign3, __ign4;
> +
> +  asm volatile ("call *%[callno]"
> + : "=a" (__res), "=b" (__ign0), "=c" (__ign1), "=d" (__ign2),
> +   "=S" (__ign3), "=D" (__ign4)
> + : "1" (a0), "2" (a1), "3" (a2), "4" (a3), "5" (a4),

I think that you can drop all __ign* variables if you specify proper
registers in input argument. If this does not work you can use "+"
modifier instead of "=" in the output argument.

> +   [callno] "a" (_page[callno * 32])
> + : "memory");
> +  return __res;
> +}

Daniel

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

Re: [Xen-devel] [PATCH v4 08/19] xen: add basic hooks for PVH in current code

[Daniel Kiper]

On Thu, Nov 08, 2018 at 08:23:20PM +0100, Juergen Gross wrote:
> On 08/11/2018 16:45, Daniel Kiper wrote:
> > On Fri, Nov 02, 2018 at 01:37:27PM +0100, Juergen Gross wrote:
> >> Add the hooks to current code needed for Xen PVH. They will be filled
> >> with code later when the related functionality is being added.
> >>
> >> Signed-off-by: Juergen Gross 
> >> ---
> >> V3: xenpvh->xen_pvh (Daniel Kiper)
> >> adjust copyright date (Roger Pau Monné)
> >> ---
> >>  grub-core/kern/i386/xen/pvh.c | 36 
> >> +++
> >>  grub-core/kern/i386/xen/startup_pvh.S | 29 
> >>  grub-core/kern/xen/init.c |  6 ++
> >>  grub-core/loader/i386/linux.c |  1 +
> >>  include/grub/i386/xen_pvh/kernel.h| 30 +
> >>  include/grub/xen.h|  6 ++
> >>  6 files changed, 108 insertions(+)
> >>  create mode 100644 grub-core/kern/i386/xen/pvh.c
> >>  create mode 100644 grub-core/kern/i386/xen/startup_pvh.S
> >>  create mode 100644 include/grub/i386/xen_pvh/kernel.h
> >>
> >> diff --git a/grub-core/kern/i386/xen/pvh.c b/grub-core/kern/i386/xen/pvh.c
> >> new file mode 100644
> >> index 0..ac6181f4e
> >> --- /dev/null
> >> +++ b/grub-core/kern/i386/xen/pvh.c
> >> @@ -0,0 +1,36 @@
> >> +/*
> >> + *  GRUB  --  GRand Unified Bootloader
> >> + *  Copyright (C) 2018  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/>.
> >> + */
> >> +
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +
> >> +grub_uint64_t grub_rsdp_addr;
> >> +
> >> +void
> >> +grub_xen_setup_pvh (void)
> >> +{
> >> +}
> >> +
> >> +grub_err_t
> >> +grub_machine_mmap_iterate (grub_memory_hook_t hook, void *hook_data)
> >> +{
> >> +}
> >> diff --git a/grub-core/kern/i386/xen/startup_pvh.S 
> >> b/grub-core/kern/i386/xen/startup_pvh.S
> >> new file mode 100644
> >> index 0..69b8fdcca
> >> --- /dev/null
> >> +++ b/grub-core/kern/i386/xen/startup_pvh.S
> >> @@ -0,0 +1,29 @@
> >> +/* startup.S - bootstrap GRUB itself */
> >> +/*
> >> + *  GRUB  --  GRand Unified Bootloader
> >> + *  Copyright (C) 2018  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/>.
> >> + */
> >> +
> >> +#include 
> >> +#include 
> >> +
> >> +  .file   "startup_pvh.S"
> >> +  .text
> >> +
> >> +/* Saved pointer to start info structure. */
> >> +  .globl  pvh_start_info
> >> +pvh_start_info:
> >> +  .long   0
> >> diff --git a/grub-core/kern/xen/init.c b/grub-core/kern/xen/init.c
> >> index 10007b411..782ca7295 100644
> >> --- a/grub-core/kern/xen/init.c
> >> +++ b/grub-core/kern/xen/init.c

Re: [Xen-devel] [PATCH v4 09/19] xen: add PVH boot entry code

[Daniel Kiper]

On Fri, Nov 02, 2018 at 01:37:28PM +0100, Juergen Gross wrote:
> Add the code for the Xen PVH mode boot entry.
>
> Signed-off-by: Juergen Gross 

One nitpick below. Otherwise
  Reviewed-by: Daniel Kiper 

> ---
> V3: clear %fs and %gs, too (Daniel Kiper)
> use GRUB_MEMORY_MACHINE_PROT_STACK_SIZE for stack size (Daniel Kiper)
> ---
>  grub-core/kern/i386/xen/startup_pvh.S | 52 
> +++
>  1 file changed, 52 insertions(+)
>
> diff --git a/grub-core/kern/i386/xen/startup_pvh.S 
> b/grub-core/kern/i386/xen/startup_pvh.S
> index 69b8fdcca..417655990 100644
> --- a/grub-core/kern/i386/xen/startup_pvh.S
> +++ b/grub-core/kern/i386/xen/startup_pvh.S
> @@ -19,11 +19,63 @@
>
>  #include 
>  #include 
> +#include 
>
>   .file   "startup_pvh.S"
>   .text
> + .globl  start, _start
> + .code32
>
> +start:
> +_start:
> + cld
> + lgdtgdtdesc
> + ljmp$GRUB_MEMORY_MACHINE_PROT_MODE_CSEG, $1f
> +1:
> + movl$GRUB_MEMORY_MACHINE_PROT_MODE_DSEG, %eax
> + mov %eax, %ds
> + mov %eax, %es
> + mov %eax, %ss
> + mov %eax, %fs
> + mov %eax, %gs

I would do this in that order:

mov %eax, %ds
mov %eax, %es
mov %eax, %fs
mov %eax, %gs
mov %eax, %ss

Daniel

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

Re: [Xen-devel] [PATCH v4 08/19] xen: add basic hooks for PVH in current code

[Daniel Kiper]

On Fri, Nov 02, 2018 at 01:37:27PM +0100, Juergen Gross wrote:
> Add the hooks to current code needed for Xen PVH. They will be filled
> with code later when the related functionality is being added.
>
> Signed-off-by: Juergen Gross 
> ---
> V3: xenpvh->xen_pvh (Daniel Kiper)
> adjust copyright date (Roger Pau Monné)
> ---
>  grub-core/kern/i386/xen/pvh.c | 36 
> +++
>  grub-core/kern/i386/xen/startup_pvh.S | 29 
>  grub-core/kern/xen/init.c |  6 ++
>  grub-core/loader/i386/linux.c |  1 +
>  include/grub/i386/xen_pvh/kernel.h| 30 +
>  include/grub/xen.h|  6 ++
>  6 files changed, 108 insertions(+)
>  create mode 100644 grub-core/kern/i386/xen/pvh.c
>  create mode 100644 grub-core/kern/i386/xen/startup_pvh.S
>  create mode 100644 include/grub/i386/xen_pvh/kernel.h
>
> diff --git a/grub-core/kern/i386/xen/pvh.c b/grub-core/kern/i386/xen/pvh.c
> new file mode 100644
> index 0..ac6181f4e
> --- /dev/null
> +++ b/grub-core/kern/i386/xen/pvh.c
> @@ -0,0 +1,36 @@
> +/*
> + *  GRUB  --  GRand Unified Bootloader
> + *  Copyright (C) 2018  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/>.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +grub_uint64_t grub_rsdp_addr;
> +
> +void
> +grub_xen_setup_pvh (void)
> +{
> +}
> +
> +grub_err_t
> +grub_machine_mmap_iterate (grub_memory_hook_t hook, void *hook_data)
> +{
> +}
> diff --git a/grub-core/kern/i386/xen/startup_pvh.S 
> b/grub-core/kern/i386/xen/startup_pvh.S
> new file mode 100644
> index 0..69b8fdcca
> --- /dev/null
> +++ b/grub-core/kern/i386/xen/startup_pvh.S
> @@ -0,0 +1,29 @@
> +/* startup.S - bootstrap GRUB itself */
> +/*
> + *  GRUB  --  GRand Unified Bootloader
> + *  Copyright (C) 2018  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/>.
> + */
> +
> +#include 
> +#include 
> +
> + .file   "startup_pvh.S"
> + .text
> +
> +/* Saved pointer to start info structure. */
> + .globl  pvh_start_info
> +pvh_start_info:
> + .long   0
> diff --git a/grub-core/kern/xen/init.c b/grub-core/kern/xen/init.c
> index 10007b411..782ca7295 100644
> --- a/grub-core/kern/xen/init.c
> +++ b/grub-core/kern/xen/init.c
> @@ -45,6 +45,8 @@ grub_xen_ptr2mfn (void *ptr)
>grub_xen_mfn_t *mfn_list =
>  (grub_xen_mfn_t *) grub_xen_start_page_addr->mfn_list;
>return mfn_list[(grub_addr_t) ptr >> GRUB_XEN_LOG_PAGE_SIZE];
> +#else
> +  return (grub_addr_t) ptr >> GRUB_XEN_LOG_PAGE_SIZE;

It seems to me that this change does not belong to this patch.

>  #endif
>  }
>
> @@ -562,6 +564,10 @@ grub_machine_init (void)
>  + GRUB_KERNEL_MACHINE_MOD_GAP,
>  GRUB_KERNEL_MACHINE_MOD_ALIGN);
>
> +#ifdef GRUB_MACHINE_XEN_PVH
> +  grub_xen_setup_pvh ();
> +#endif
> +
>grub_xen_setup_gnttab ();
>
>  #ifdef GRUB_MACHINE_XEN
> diff --git a/grub-core/loader/i386/linux.c b/grub-core/loader/i386/linux.c
> index 51920896e..f96309476 100644
> --- a/grub-core/loader/i386/linux.c
> +++ b/grub-core/loader/i386/linux.c
> @@ -35,6 +35,7 @@
>  #include 
>  #include 
>  #include 
> +#include 

Please say in the comm

Re: [Xen-devel] [PATCH v4 05/19] xen: add some dummy headers for PVH mode

[Daniel Kiper]

On Wed, Nov 07, 2018 at 03:49:51PM +0100, Juergen Gross wrote:
> On 07/11/2018 13:21, Daniel Kiper wrote:
> > On Fri, Nov 02, 2018 at 01:37:24PM +0100, Juergen Gross wrote:
> >> With Xen PVH mode adding a new machine type the machine related headers
> >> need to be present for the build to succeed. Most of the headers just
> >> need to include the related common i386 headers. Add those to the tree.
> >>
> >> Signed-off-by: Juergen Gross 
> >> ---
> >> V3: updated commit message (Daniel Kiper)
> >> xenpvh->xen_pvh (Daniel Kiper)
> >> ---
> >>  include/grub/i386/xen_pvh/boot.h| 1 +
> >>  include/grub/i386/xen_pvh/console.h | 1 +
> >>  include/grub/i386/xen_pvh/int.h | 1 +
> >>  include/grub/i386/xen_pvh/memory.h  | 1 +
> >>  include/grub/i386/xen_pvh/time.h| 1 +
> >>  5 files changed, 5 insertions(+)
> >>  create mode 100644 include/grub/i386/xen_pvh/boot.h
> >>  create mode 100644 include/grub/i386/xen_pvh/console.h
> >>  create mode 100644 include/grub/i386/xen_pvh/int.h
> >>  create mode 100644 include/grub/i386/xen_pvh/memory.h
> >>  create mode 100644 include/grub/i386/xen_pvh/time.h
> >>
> >> diff --git a/include/grub/i386/xen_pvh/boot.h 
> >> b/include/grub/i386/xen_pvh/boot.h
> >> new file mode 100644
> >> index 0..6cd23aa83
> >> --- /dev/null
> >> +++ b/include/grub/i386/xen_pvh/boot.h
> >> @@ -0,0 +1 @@
> >> +#include 
> >> diff --git a/include/grub/i386/xen_pvh/console.h 
> >> b/include/grub/i386/xen_pvh/console.h
> >> new file mode 100644
> >> index 0..305a46d8e
> >> --- /dev/null
> >> +++ b/include/grub/i386/xen_pvh/console.h
> >> @@ -0,0 +1 @@
> >> +#include 
> >> diff --git a/include/grub/i386/xen_pvh/int.h 
> >> b/include/grub/i386/xen_pvh/int.h
> >> new file mode 100644
> >> index 0..0f1f9ee62
> >> --- /dev/null
> >> +++ b/include/grub/i386/xen_pvh/int.h
> >> @@ -0,0 +1 @@
> >> +#include 
> >
> > I think that this begs for explanation in the commit message
> > why not grub/i386/pc/int.h.
>
> Okay.
>
> >
> >> diff --git a/include/grub/i386/xen_pvh/memory.h 
> >> b/include/grub/i386/xen_pvh/memory.h
> >> new file mode 100644
> >> index 0..8dd6f7c8c
> >> --- /dev/null
> >> +++ b/include/grub/i386/xen_pvh/memory.h
> >> @@ -0,0 +1 @@
> >> +#include 
> >
> > Hmmm... Why not include/grub/i386/pc/memory.h?
>
> The coreboot variant is containing the stubs grub_machine_mmap_register
> and grub_machine_mmap_unregister I need, with the pc variant I'd have to
> add those to xen-pvh code.
>
> Using the coreboot variant for that purpose seems to be common practice:
> include/grub/i386/qemu/memory.h
> include/grub/i386/ieee1275/memory.h
> include/grub/i386/multiboot/memory.h
> are doing exactly the same.

I am OK with it then. However, please say about that in the commit
message. Otherwise it looks like a mistake.

Daniel

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

Re: [Xen-devel] x86 Community Call: Nov 14 - 15:00 - 16:00 UTC - Call for Agenda Items

[Daniel Kiper]

On Wed, Nov 07, 2018 at 04:15:17PM +0100, Juergen Gross wrote:
> On 07/11/2018 16:06, Daniel Kiper wrote:
> > On Wed, Nov 07, 2018 at 11:49:38AM +0100, Daniel Kiper wrote:
> >> On Tue, Nov 06, 2018 at 09:54:54AM -0700, Jan Beulich wrote:
> >>>>>> On 02.11.18 at 18:59,  wrote:
> >>>> It’s time again for the x86 community call: for the agenda see
> >>>> https://docs.google.com/document/d/1RxW-iwcFFuKzNjjEqLEtiwFVHgAUlk35c0EtTkRE1
> >>>> k4/edit#
> >>>>
> >>>> Please propose new agenda items by next Friday (feel free to just add 
> >>>> them
> >>>> to the document or reply to this mail)
> >>>
> >>> I've just added a TMEM item, and seeing only Daniel from Oracle on the
> >>> Cc list here I've added Konrad so he might arrange for someone to
> >>> attend to clarify their intentions with it.
> >>
> >> Konrad is not available this week. However, AFAICT we do not have any
> >> plans WRT TMEM. Anyway, I am CC-ing Joao and Boris. There is chance that
> >> they know something more than I.
> >
> > Konrad told me that if you would like to rip TMEM out he will ack the patch.
>
> Is this true for Linux kernel drivers/xen/tmem.c, too?

Yep!

Daniel

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

Re: [Xen-devel] x86 Community Call: Nov 14 - 15:00 - 16:00 UTC - Call for Agenda Items

[Daniel Kiper]

On Wed, Nov 07, 2018 at 11:49:38AM +0100, Daniel Kiper wrote:
> On Tue, Nov 06, 2018 at 09:54:54AM -0700, Jan Beulich wrote:
> > >>> On 02.11.18 at 18:59,  wrote:
> > > It’s time again for the x86 community call: for the agenda see
> > > https://docs.google.com/document/d/1RxW-iwcFFuKzNjjEqLEtiwFVHgAUlk35c0EtTkRE1
> > > k4/edit#
> > >
> > > Please propose new agenda items by next Friday (feel free to just add them
> > > to the document or reply to this mail)
> >
> > I've just added a TMEM item, and seeing only Daniel from Oracle on the
> > Cc list here I've added Konrad so he might arrange for someone to
> > attend to clarify their intentions with it.
>
> Konrad is not available this week. However, AFAICT we do not have any
> plans WRT TMEM. Anyway, I am CC-ing Joao and Boris. There is chance that
> they know something more than I.

Konrad told me that if you would like to rip TMEM out he will ack the patch.

Daniel

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

Re: [Xen-devel] [PATCH v4 07/19] xen: add PVH specific defines to offset.h

[Daniel Kiper]

On Fri, Nov 02, 2018 at 01:37:26PM +0100, Juergen Gross wrote:
> include/grub/offsets.h needs some defines for Xen PVH mode.
>
> Add them. While at it line up the values in the surrounding lines to
> start at the same column.
>
> Signed-off-by: Juergen Gross 

Reviewed-by: Daniel Kiper 

Daniel

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

Re: [Xen-devel] [PATCH v4 05/19] xen: add some dummy headers for PVH mode

[Daniel Kiper]

On Fri, Nov 02, 2018 at 01:37:24PM +0100, Juergen Gross wrote:
> With Xen PVH mode adding a new machine type the machine related headers
> need to be present for the build to succeed. Most of the headers just
> need to include the related common i386 headers. Add those to the tree.
>
> Signed-off-by: Juergen Gross 
> ---
> V3: updated commit message (Daniel Kiper)
> xenpvh->xen_pvh (Daniel Kiper)
> ---
>  include/grub/i386/xen_pvh/boot.h| 1 +
>  include/grub/i386/xen_pvh/console.h | 1 +
>  include/grub/i386/xen_pvh/int.h | 1 +
>  include/grub/i386/xen_pvh/memory.h  | 1 +
>  include/grub/i386/xen_pvh/time.h| 1 +
>  5 files changed, 5 insertions(+)
>  create mode 100644 include/grub/i386/xen_pvh/boot.h
>  create mode 100644 include/grub/i386/xen_pvh/console.h
>  create mode 100644 include/grub/i386/xen_pvh/int.h
>  create mode 100644 include/grub/i386/xen_pvh/memory.h
>  create mode 100644 include/grub/i386/xen_pvh/time.h
>
> diff --git a/include/grub/i386/xen_pvh/boot.h 
> b/include/grub/i386/xen_pvh/boot.h
> new file mode 100644
> index 0..6cd23aa83
> --- /dev/null
> +++ b/include/grub/i386/xen_pvh/boot.h
> @@ -0,0 +1 @@
> +#include 
> diff --git a/include/grub/i386/xen_pvh/console.h 
> b/include/grub/i386/xen_pvh/console.h
> new file mode 100644
> index 0..305a46d8e
> --- /dev/null
> +++ b/include/grub/i386/xen_pvh/console.h
> @@ -0,0 +1 @@
> +#include 
> diff --git a/include/grub/i386/xen_pvh/int.h b/include/grub/i386/xen_pvh/int.h
> new file mode 100644
> index 0..0f1f9ee62
> --- /dev/null
> +++ b/include/grub/i386/xen_pvh/int.h
> @@ -0,0 +1 @@
> +#include 

I think that this begs for explanation in the commit message
why not grub/i386/pc/int.h.

> diff --git a/include/grub/i386/xen_pvh/memory.h 
> b/include/grub/i386/xen_pvh/memory.h
> new file mode 100644
> index 0..8dd6f7c8c
> --- /dev/null
> +++ b/include/grub/i386/xen_pvh/memory.h
> @@ -0,0 +1 @@
> +#include 

Hmmm... Why not include/grub/i386/pc/memory.h?

> diff --git a/include/grub/i386/xen_pvh/time.h 
> b/include/grub/i386/xen_pvh/time.h
> new file mode 100644
> index 0..2298ee8f4
> --- /dev/null
> +++ b/include/grub/i386/xen_pvh/time.h
> @@ -0,0 +1 @@
> +#include 

Daniel

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

Re: [Xen-devel] [PATCH v4 04/19] xen: prepare common code for Xen PVH support

[Daniel Kiper]

On Fri, Nov 02, 2018 at 01:37:23PM +0100, Juergen Gross wrote:
> Some common code needs to be special cased for Xen PVH mode. This hits
> mostly Xen PV mode specific areas.
>
> Split include/grub/i386/pc/int_types.h off from
> include/grub/i386/pc/int.h to support including this file later from
> xen_pvh code without the grub_bios_interrupt definition.
>
> Move definition of struct grub_e820_mmap_entry from
> grub-core/mmap/i386/pc/mmap.c to include/grub/i386/memory.h in order
> to make it usable from xen_pvh code.
>
> Signed-off-by: Juergen Gross 

If you fix two nitpicks below you can add
  Reviewed-by: Daniel Kiper 

> ---
> V3: GRUB_MACHINE_XENPVH -> GRUB_MACHINE_XEN_PVH (Daniel Kiper)
>     split include/grub/i386/pc/int.h (Daniel Kiper)
> move struct grub_e820_mmap_entry definition to header file
> ---
>  grub-core/kern/i386/tsc.c |  2 +-
>  grub-core/mmap/i386/pc/mmap.c |  7 -
>  include/grub/i386/memory.h|  7 +
>  include/grub/i386/pc/int.h| 36 +---
>  include/grub/i386/pc/int_types.h  | 59 
> +++
>  include/grub/i386/tsc.h   |  2 +-
>  include/grub/i386/xen/hypercall.h |  5 +++-
>  include/grub/kernel.h |  4 ++-
>  8 files changed, 76 insertions(+), 46 deletions(-)
>  create mode 100644 include/grub/i386/pc/int_types.h
>
> diff --git a/grub-core/kern/i386/tsc.c b/grub-core/kern/i386/tsc.c
> index f266eb131..9293b161d 100644
> --- a/grub-core/kern/i386/tsc.c
> +++ b/grub-core/kern/i386/tsc.c
> @@ -65,7 +65,7 @@ grub_tsc_init (void)
>
>tsc_boot_time = grub_get_tsc ();
>
> -#ifdef GRUB_MACHINE_XEN
> +#if defined (GRUB_MACHINE_XEN) || defined (GRUB_MACHINE_XEN_PVH)
>(void) (grub_tsc_calibrate_from_xen () || calibrate_tsc_hardcode());
>  #elif defined (GRUB_MACHINE_EFI)
>(void) (grub_tsc_calibrate_from_pmtimer () || grub_tsc_calibrate_from_pit 
> () || grub_tsc_calibrate_from_efi() || calibrate_tsc_hardcode());
> diff --git a/grub-core/mmap/i386/pc/mmap.c b/grub-core/mmap/i386/pc/mmap.c
> index 609994516..bcb097c38 100644
> --- a/grub-core/mmap/i386/pc/mmap.c
> +++ b/grub-core/mmap/i386/pc/mmap.c
> @@ -42,13 +42,6 @@ extern grub_uint16_t grub_machine_mmaphook_kblow;
>  extern grub_uint16_t grub_machine_mmaphook_kbin16mb;
>  extern grub_uint16_t grub_machine_mmaphook_64kbin4gb;
>
> -struct grub_e820_mmap_entry
> -{
> -  grub_uint64_t addr;
> -  grub_uint64_t len;
> -  grub_uint32_t type;
> -} GRUB_PACKED;
> -
>

Please drop this extra empty line too.

[...]

> diff --git a/include/grub/i386/pc/int_types.h 
> b/include/grub/i386/pc/int_types.h
> new file mode 100644
> index 0..35a4b5087
> --- /dev/null
> +++ b/include/grub/i386/pc/int_types.h
> @@ -0,0 +1,59 @@
> +/*
> + *  GRUB  --  GRand Unified Bootloader
> + *  Copyright (C) 2018  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/>.
> + */
> +
> +#ifndef GRUB_INTERRUPT_TYPES_MACHINE_HEADER
> +#define GRUB_INTERRUPT_TYPES_MACHINE_HEADER  1
> +
> +#include 
> +
> +struct grub_bios_int_registers
> +{
> +  grub_uint32_t eax;
> +  grub_uint16_t es;
> +  grub_uint16_t ds;
> +  grub_uint16_t flags;
> +  grub_uint16_t dummy;
> +  grub_uint32_t ebx;
> +  grub_uint32_t ecx;
> +  grub_uint32_t edi;
> +  grub_uint32_t esi;
> +  grub_uint32_t edx;
> +};

Please move this struct behind constants definitions below.

> +#define  GRUB_CPU_INT_FLAGS_CARRY 0x1
> +#define  GRUB_CPU_INT_FLAGS_PARITY0x4
> +#define  GRUB_CPU_INT_FLAGS_ADJUST0x10
> +#define  GRUB_CPU_INT_FLAGS_ZERO  0x40
> +#define  GRUB_CPU_INT_FLAGS_SIGN  0x80
> +#define  GRUB_CPU_INT_FLAGS_TRAP  0x100
> +#define  GRUB_CPU_INT_FLAGS_INTERRUPT 0x200
> +#define  GRUB_CPU_INT_FLAGS_DIRECTION 0x400
> +#define  GRUB_CPU_INT_FLAGS_OVERFLOW  0x800
> +#ifdef GRUB_MACHINE_PCBIOS
> +#define  GRUB_CPU_INT_FLAGS_DEFAULT   GRUB_CPU_INT_FLAGS_INTERRUPT
> +#else
> +#define  GRUB_CPU_INT_FLAGS_DEFAULT   0
> +#endif
> +
> +struct grub_i386_idt
> +{
> +  grub_uint16_t limit;
> +  grub_uint32_t base;
> +} GRUB_PACKED;
> +
> +#endif

Daniel

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

Re: [Xen-devel] [PATCH v4 02/19] loader/linux: support passing rsdp address via boot params

[Daniel Kiper]

On Fri, Nov 02, 2018 at 01:37:21PM +0100, Juergen Gross wrote:
> Xen PVH guests will have the RSDP at an arbitrary address. Support that
> by passing the RSDP address via the boot parameters to Linux.
>
> The new protocol version 2.14 requires to set version to 0x8000 ored
> with the actually use protocol version (the minimum of the kernel
> supplied protocol version and the grub2 supported protocol version)
> if 2.14 or higher are in effect.
>
> Signed-off-by: Juergen Gross 

Reviewed-by: Daniel Kiper 

Daniel

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

Re: [Xen-devel] [PATCH v2 13/18] xen: init memory regions for PVH

[Daniel Kiper]

On Mon, Oct 22, 2018 at 01:43:53PM +0200, Juergen Gross wrote:
> On 22/10/2018 13:31, Daniel Kiper wrote:
> > On Tue, Oct 09, 2018 at 01:03:12PM +0200, Juergen Gross wrote:
> >> Add all usable memory regions to grub memory management and add the
> >> needed mmap iterate code.
> >
> > I am missing a few words why this patch is needed. Especially why
> > grub_machine_mmap_iterate() has to belong to this patch. However,
> > I think that it should be introduced by patch in which
> > grub_machine_mmap_iterate() is used at some point.
>
> That would again lead to one giant PVH patch which you didn't like.
>
> grub_machine_mmap_iterate() is being used by grub common code like
> grub-core/lib/relocator.c or grub-core/mmap/mmap.c
>
> grub_machine_mmap_iterate() belongs into this patch as it is the
> main user of the memory map introduced here.

OK, let's leave it here. Though commit message has to be updated accordingly.

Daniel

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

Re: [Xen-devel] [PATCH v2 18/18] xenpvh: add support to configure

[Daniel Kiper]

On Tue, Oct 09, 2018 at 01:03:17PM +0200, Juergen Gross wrote:
> Support platform i386/xenpvh in configure.
>
> Signed-off-by: Juergen Gross 
> Reviewed-by: Daniel Kiper 

+/- XENPVH/xenpvh play still Reviewed-by: Daniel Kiper 

Daniel

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

Re: [Xen-devel] [PATCH v2 17/18] xenpvh: support grub-install for xenpvh

[Daniel Kiper]

On Tue, Oct 09, 2018 at 01:03:16PM +0200, Juergen Gross wrote:
> Add xenpvh support to grub-install.
>
> Signed-off-by: Juergen Gross 
> Reviewed-by: Daniel Kiper 

+/- XENPVH play still Reviewed-by: Daniel Kiper 

Daniel

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

Re: [Xen-devel] [PATCH v2 16/18] xenpvh: support building a standalone image

[Daniel Kiper]

On Tue, Oct 09, 2018 at 01:03:15PM +0200, Juergen Gross wrote:
> Support mkimage for xenpvh.
>
> In order to avoid using plain integers for the ELF notes use the
> available Xen include instead. While at it replace the plain numbers
> for Xen PV mode, too.
>
> Signed-off-by: Juergen Gross 

+/- XENPVH/xenpvh play Reviewed-by: Daniel Kiper 

Daniel

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

Re: [Xen-devel] [PATCH v2 15/18] grub-module-verifier: Ignore all_video for xenpvh

[Daniel Kiper]

On Tue, Oct 09, 2018 at 01:03:14PM +0200, Juergen Gross wrote:
> From: Hans van Kranenburg 
>
> This solves the build failing with "Error: no symbol table and no
> .moddeps section"
>
> Also see:
> - 6371e9c10433578bb236a8284ddb9ce9e201eb59
> - https://savannah.gnu.org/bugs/?49012
>
> Signed-off-by: Hans van Kranenburg 

Except s/xenpvh/xen_pvh/ Reviewed-by: Daniel Kiper 

Daniel

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

Re: [Xen-devel] [PATCH v2 14/18] xenpvh: add build runes for grub-core

[Daniel Kiper]

On Tue, Oct 09, 2018 at 01:03:13PM +0200, Juergen Gross wrote:
> Add the modifications to the build system needed to build a xenpvh
> grub.
>
> Signed-off-by: Juergen Gross 
> Reviewed-by: Daniel Kiper 
> ---
>  gentpl.py   |  4 ++--
>  grub-core/Makefile.am   | 12 
>  grub-core/Makefile.core.def | 35 +++
>  3 files changed, 49 insertions(+), 2 deletions(-)
>
> diff --git a/gentpl.py b/gentpl.py
> index da67965a4..9732b4aee 100644
> --- a/gentpl.py
> +++ b/gentpl.py
> @@ -28,7 +28,7 @@ import re
>
>  GRUB_PLATFORMS = [ "emu", "i386_pc", "i386_efi", "i386_qemu", 
> "i386_coreboot",
> "i386_multiboot", "i386_ieee1275", "x86_64_efi",
> -   "i386_xen", "x86_64_xen",
> +   "i386_xen", "x86_64_xen", "i386_xenpvh",

s/i386_xenpvh/i386_xen_pvh/ here and below please...

> "mips_loongson", "sparc64_ieee1275",
> "powerpc_ieee1275", "mips_arc", "ia64_efi",
> "mips_qemu_mips", "arm_uboot", "arm_efi", "arm64_efi",
> @@ -71,7 +71,7 @@ GROUPS["videomodules"]   = GRUB_PLATFORMS[:];
>  for i in GROUPS["videoinkernel"]: GROUPS["videomodules"].remove(i)
>
>  # Similar for terminfo
> -GROUPS["terminfoinkernel"] = [ "emu", "mips_loongson", "mips_arc", 
> "mips_qemu_mips" ] + GROUPS["xen"] + GROUPS["ieee1275"] + GROUPS["uboot"];
> +GROUPS["terminfoinkernel"] = [ "emu", "mips_loongson", "mips_arc", 
> "mips_qemu_mips", "i386_xenpvh" ] + GROUPS["xen"] + GROUPS["ieee1275"] + 
> GROUPS["uboot"];
>  GROUPS["terminfomodule"]   = GRUB_PLATFORMS[:];
>  for i in GROUPS["terminfoinkernel"]: GROUPS["terminfomodule"].remove(i)
>
> diff --git a/grub-core/Makefile.am b/grub-core/Makefile.am
> index f4ff62b76..d4417e2c4 100644
> --- a/grub-core/Makefile.am
> +++ b/grub-core/Makefile.am
> @@ -101,6 +101,18 @@ KERNEL_HEADER_FILES += 
> $(top_builddir)/include/grub/machine/int.h
>  KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/i386/tsc.h
>  endif
>
> +if COND_i386_xenpvh
> +KERNEL_HEADER_FILES += $(top_builddir)/include/grub/machine/kernel.h
> +KERNEL_HEADER_FILES += $(top_builddir)/include/grub/machine/int.h
> +KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/i386/tsc.h
> +KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/terminfo.h
> +KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/extcmd.h
> +KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/loader.h
> +KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/lib/arg.h
> +KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/xen.h
> +KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/i386/xen/hypercall.h
> +endif
> +
>  if COND_i386_efi
>  KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/efi/efi.h
>  KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/efi/disk.h
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index 9590e87d9..c42cebc38 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -79,6 +79,8 @@ kernel = {
>i386_xen_ldflags = '$(TARGET_IMG_BASE_LDOPT),0';
>x86_64_xen_ldflags   = '$(TARGET_IMG_LDFLAGS)';
>x86_64_xen_ldflags   = '$(TARGET_IMG_BASE_LDOPT),0';
> +  i386_xenpvh_ldflags  = '$(TARGET_IMG_LDFLAGS)';
> +  i386_xenpvh_ldflags  = '$(TARGET_IMG_BASE_LDOPT),0x10';
>
>mips_loongson_ldflags= '-Wl,-Ttext,0x8020';
>powerpc_ieee1275_ldflags = '-Wl,-Ttext,0x20';
> @@ -100,6 +102,7 @@ kernel = {
>x86_64_efi_startup = kern/x86_64/efi/startup.S;
>i386_xen_startup = kern/i386/xen/startup.S;
>x86_64_xen_startup = kern/x86_64/xen/startup.S;
> +  i386_xenpvh_startup = kern/i386/xen/startup_pvh.S;
>i386_qemu_startup = kern/i386/qemu/startup.S;
>i386_ieee1275_startup = kern/i386/ieee1275/startup.S;
>i386_coreboot_startup = kern/i386/coreboot/startup.S;
> @@ -177,6 +180,7 @@ kernel = {
>
>i386 = kern/i386/dl.c;
>i386_xen = kern/i386/dl.c;
> +  i386_xenpvh = kern/i386/dl.c;
>
>i386_coreboot = kern/i386/coreboot/init.c;
>i386_multiboot = kern/i386/coreboot/init.c;
> @@ -222,6 +226,14 @@ kernel = {
>xen = disk/xen/xendisk.c;
>xen = commands/boot.c;
>
> +  i386_xenpvh = kern/i386/tsc.c;
> +  i386_xenpvh = kern/i386/xen/tsc.c;
> +  i386_xenpvh = commands/boot.c;
> +  i386_xenpvh = kern/xen/init.c;
> +  i386_xenpvh = kern/i386/xen/pvh.c;
> +  i386_xenpvh = term/xen/console.c;
> +  i386_xenpvh = disk/xen/xendisk.c;

I would sort all file names here.

Daniel

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

Re: [Xen-devel] [PATCH v2 13/18] xen: init memory regions for PVH

[Daniel Kiper]

On Tue, Oct 09, 2018 at 01:03:12PM +0200, Juergen Gross wrote:
> Add all usable memory regions to grub memory management and add the
> needed mmap iterate code.

I am missing a few words why this patch is needed. Especially why
grub_machine_mmap_iterate() has to belong to this patch. However,
I think that it should be introduced by patch in which
grub_machine_mmap_iterate() is used at some point.

> As we are running in 32-bit mode don't add memory above 4GB.
>
> Signed-off-by: Juergen Gross 
> ---
>  grub-core/kern/i386/xen/pvh.c | 35 +++
>  1 file changed, 35 insertions(+)
>
> diff --git a/grub-core/kern/i386/xen/pvh.c b/grub-core/kern/i386/xen/pvh.c
> index 93ed68245..c4a8bccf4 100644
> --- a/grub-core/kern/i386/xen/pvh.c
> +++ b/grub-core/kern/i386/xen/pvh.c
> @@ -222,6 +222,30 @@ grub_xen_get_mmap (void)
>grub_xen_sort_mmap ();
>  }
>
> +static void
> +grub_xen_mm_init_regions (void)
> +{
> +  grub_uint64_t modend, from, to;
> +  unsigned int i;
> +
> +  modend = grub_modules_get_end ();
> +
> +  for (i = 0; i < nr_map_entries; i++)
> +{
> +  if (map[i].type != GRUB_MEMORY_AVAILABLE)
> +continue;
> +  from = map[i].addr;
> +  to = from + map[i].len;
> +  if (from < modend)
> +from = modend;
> +  if (from >= to || from >= 0x1ULL)
> +continue;
> +  if (to > 0x1ULL)
> +to = 0x1ULL;
> +  grub_mm_init_region ((void *) (grub_addr_t) from, to - from);
> +}
> +}
> +
>  static grub_uint64_t
>  grub_xen_find_page (grub_uint64_t start)
>  {
> @@ -302,10 +326,21 @@ grub_xen_setup_pvh (void)
>grub_xen_shared_info = grub_xen_add_physmap (XENMAPSPACE_shared_info,
>  (void *) par);
>
> +  grub_xen_mm_init_regions ();
> +
>grub_rsdp_addr = pvh_start_info->rsdp_paddr;
>  }
>
>  grub_err_t
>  grub_machine_mmap_iterate (grub_memory_hook_t hook, void *hook_data)
>  {
> +  unsigned int i;
> +
> +  for (i = 0; i < nr_map_entries; i++)
> +{
> +  if (map[i].len && hook (map[i].addr, map[i].len, map[i].type, 
> hook_data))
> +break;
> +}
> +
> +  return GRUB_ERR_NONE;

Daniel

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

Re: [Xen-devel] [PATCH v2 09/18] xen: add PVH boot entry code

[Daniel Kiper]

On Fri, Oct 19, 2018 at 04:50:25PM +0200, Juergen Gross wrote:
> On 19/10/2018 14:17, Daniel Kiper wrote:
> > On Tue, Oct 09, 2018 at 01:03:08PM +0200, Juergen Gross wrote:
> >> Add the code for the Xen PVH mode boot entry.
> >>
> >> Signed-off-by: Juergen Gross 
> >> ---
> >>  grub-core/kern/i386/xen/startup_pvh.S | 50 
> >> +++
> >>  1 file changed, 50 insertions(+)
> >>
> >> diff --git a/grub-core/kern/i386/xen/startup_pvh.S 
> >> b/grub-core/kern/i386/xen/startup_pvh.S
> >> index e18ee5b31..0ddb63b31 100644
> >> --- a/grub-core/kern/i386/xen/startup_pvh.S
> >> +++ b/grub-core/kern/i386/xen/startup_pvh.S
> >> @@ -19,11 +19,61 @@
> >>
> >>  #include 
> >>  #include 
> >> +#include 
> >>
> >>.file   "startup_pvh.S"
> >>.text
> >> +  .globl  start, _start
> >> +  .code32
> >>
> >> +start:
> >> +_start:
> >> +  cld
> >> +  lgdtgdtdesc
> >> +  ljmp$GRUB_MEMORY_MACHINE_PROT_MODE_CSEG, $1f
> >> +1:
> >> +  movl$GRUB_MEMORY_MACHINE_PROT_MODE_DSEG, %eax
> >> +  mov %eax, %ds
> >> +  mov %eax, %es
> >> +  mov %eax, %ss
> >
> > Should not you load null descriptor into %fs and %gs?
> > Just in case...
>
> Hmm, if you want I can do that, sure.

Please do.

> >> +  lealLOCAL(stack_end), %esp
> >> +
> >> +  /* Save address of start info structure. */
> >> +  mov %ebx, pvh_start_info
> >> +  callEXT_C(grub_main)
> >> +  /* Doesn't return. */
> >> +
> >> +  .p2align3
> >> +gdt:
> >> +  .word   0, 0
> >> +  .byte   0, 0, 0, 0
> >> +
> >> +  /* -- code segment --
> >> +   * base = 0x, limit = 0xF (4 KiB Granularity), present
> >> +   * type = 32bit code execute/read, DPL = 0
> >> +   */
> >> +  .word   0x, 0
> >> +  .byte   0, 0x9A, 0xCF, 0
> >> +
> >> +  /* -- data segment --
> >> +   * base = 0x, limit 0xF (4 KiB Granularity), present
> >> +   * type = 32 bit data read/write, DPL = 0
> >> +   */
> >> +  .word   0x, 0
> >> +  .byte   0, 0x92, 0xCF, 0
> >> +
> >> +  .p2align3
> >> +/* this is the GDT descriptor */
> >> +gdtdesc:
> >> +  .word   0x17/* limit */
> >> +  .long   gdt /* addr */
> >> +
> >> +  .p2align2
> >>  /* Saved pointer to start info structure. */
> >>.globl  pvh_start_info
> >>  pvh_start_info:
> >>.long   0
> >> +
> >> +  .bss
> >> +  .space  (1 << 22)
> >
> > Hmmm... Why do we need 4 MiB here? If this is really needed then it begs for
> > a comment. And I would like to see a constant instead of plain number here.
>
> This is just copied from xen/startup.S
>
> I can reduce it to something near GRUB_MEMORY_MACHINE_PROT_STACK_SIZE
> (about 64kB).

GRUB_MEMORY_MACHINE_PROT_STACK_SIZE works for me.

Daniel

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

Re: [Xen-devel] [PATCH v2 08/18] xen: add basic hooks for PVH in current code

[Daniel Kiper]

On Fri, Oct 19, 2018 at 05:52:44PM +0200, Juergen Gross wrote:
> On 19/10/2018 17:33, Roger Pau Monné wrote:
> > On Tue, Oct 09, 2018 at 01:03:07PM +0200, Juergen Gross wrote:
> >> Add the hooks to current code needed for Xen PVH.
> >>
> >> Signed-off-by: Juergen Gross 
> >> ---
> >>  grub-core/kern/i386/xen/pvh.c | 36 
> >> +++
> >>  grub-core/kern/i386/xen/startup_pvh.S | 29 
> >>  grub-core/kern/xen/init.c |  6 ++
> >>  include/grub/i386/xenpvh/kernel.h | 30 +
> >>  include/grub/xen.h|  6 ++
> >>  5 files changed, 107 insertions(+)
> >>  create mode 100644 grub-core/kern/i386/xen/pvh.c
> >>  create mode 100644 grub-core/kern/i386/xen/startup_pvh.S
> >>  create mode 100644 include/grub/i386/xenpvh/kernel.h
> >>
> >> diff --git a/grub-core/kern/i386/xen/pvh.c b/grub-core/kern/i386/xen/pvh.c
> >> new file mode 100644
> >> index 0..182ef95f9
> >> --- /dev/null
> >> +++ b/grub-core/kern/i386/xen/pvh.c
> >> @@ -0,0 +1,36 @@
> >> +/*
> >> + *  GRUB  --  GRand Unified Bootloader
> >> + *  Copyright (C) 2011  Free Software Foundation, Inc.
> >
> > Isn't this header (and the ones below) kind of off at least year
> > wise?
>
> Hmm, only a little bit :-)
>
> Will update.

Please do this for all headers in the patchset

Daniel

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

Re: [Xen-devel] [PATCH v2 12/18] xen: setup Xen specific data for PVH

[Daniel Kiper]

On Tue, Oct 09, 2018 at 01:03:11PM +0200, Juergen Gross wrote:
> Initialize the needed Xen specific data. This is:
>
> - the Xen start of day page containing the console and Xenstore ring
>   page PFN and event channel
> - the grant table
> - the shared info page
>
> Set the RSDP address for the guest from the start_info page passed
> as boot parameter.
>
> Signed-off-by: Juergen Gross 
> ---
>  grub-core/kern/i386/xen/pvh.c | 107 
> ++
>  1 file changed, 107 insertions(+)
>
> diff --git a/grub-core/kern/i386/xen/pvh.c b/grub-core/kern/i386/xen/pvh.c
> index b4933b454..93ed68245 100644
> --- a/grub-core/kern/i386/xen/pvh.c
> +++ b/grub-core/kern/i386/xen/pvh.c
> @@ -24,6 +24,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>
>  struct xen_machine_mmap_entry
> @@ -39,6 +40,7 @@ static struct { char _entry[32]; } hypercall_page[128]
>__attribute__ ((aligned (GRUB_XEN_PAGE_SIZE)));
>
>  static grub_uint32_t xen_cpuid_base;
> +static struct start_info grub_xen_start_page;
>  static struct xen_machine_mmap_entry map[128];
>  static unsigned int nr_map_entries;
>
> @@ -104,6 +106,36 @@ grub_xen_hypercall (grub_uint32_t callno, grub_uint32_t 
> a0,
>return __res;
>  }
>
> +static grub_uint32_t
> +grub_xen_get_param (int idx)
> +{
> +  struct xen_hvm_param xhv;
> +  int r;
> +
> +  xhv.domid = DOMID_SELF;
> +  xhv.index = idx;
> +  r = grub_xen_hypercall (__HYPERVISOR_hvm_op, HVMOP_get_param,
> +   (grub_uint32_t) (), 0, 0, 0, 0);

s/(grub_uint32_t) ()/(grub_uint32_t)()/
Here and in the other patches...

> +  if (r < 0)
> +grub_xen_early_halt ();
> +  return xhv.value;
> +}
> +
> +static void *
> +grub_xen_add_physmap (unsigned int space, void *addr)
> +{
> +  struct xen_add_to_physmap xatp;
> +
> +  xatp.domid = DOMID_SELF;
> +  xatp.idx = 0;
> +  xatp.space = space;
> +  xatp.gpfn = (grub_addr_t) addr >> GRUB_XEN_LOG_PAGE_SIZE;
> +  if (grub_xen_hypercall (__HYPERVISOR_memory_op, XENMEM_add_to_physmap,
> +   (grub_uint32_t) (), 0, 0, 0, 0))
> +grub_xen_early_halt ();
> +  return addr;
> +}
> +
>  static void
>  grub_xen_sort_mmap (void)
>  {
> @@ -190,12 +222,87 @@ grub_xen_get_mmap (void)
>grub_xen_sort_mmap ();
>  }
>
> +static grub_uint64_t
> +grub_xen_find_page (grub_uint64_t start)
> +{
> +  unsigned int i, j;
> +  grub_uint64_t last = start;
> +
> +  /* Try to find a e820 map hole below 4G. */
> +  for (i = 0; i < nr_map_entries; i++)
> +{
> +  if (last > map[i].addr + map[i].len)
> + continue;
> +  if (last < map[i].addr)
> + return last;
> +  if ((map[i].addr >> 32) || ((map[i].addr + map[i].len) >> 32))
> + break;
> +  last = map[i].addr + map[i].len;
> +}
> +if (i == nr_map_entries)
> +  return last;
> +
> +  /* No hole found, use the highest RAM page below 4G and reserve it. */

It seems to me that this comment should be put before next for().

> +  if (nr_map_entries == ARRAY_SIZE(map))
> +grub_xen_early_halt ();
> +  j = 0;
> +  for (i = 0; i < nr_map_entries; i++)

for (i = 0, j = 0; i < nr_map_entries; i++)

Daniel

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

Re: [Xen-devel] [PATCH v2 11/18] xen: get memory map from hypervisor for PVH

[Daniel Kiper]

On Tue, Oct 09, 2018 at 01:03:10PM +0200, Juergen Gross wrote:
> Retrieve the memory map from the hypervisor and normalize it to contain
> no overlapping entries and to be sorted by address.
>
> Signed-off-by: Juergen Gross 
> ---
>  grub-core/kern/i386/xen/pvh.c | 98 
> +++
>  1 file changed, 98 insertions(+)
>
> diff --git a/grub-core/kern/i386/xen/pvh.c b/grub-core/kern/i386/xen/pvh.c
> index c1b1cf8db..b4933b454 100644
> --- a/grub-core/kern/i386/xen/pvh.c
> +++ b/grub-core/kern/i386/xen/pvh.c
> @@ -22,7 +22,16 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
> +#include 
> +
> +struct xen_machine_mmap_entry
> +{
> +  grub_uint64_t addr;
> +  grub_uint64_t len;
> +  grub_uint32_t type;
> +} GRUB_PACKED;

Could not we reuse grub_e820_mmap_entry here?

>  grub_uint64_t grub_rsdp_addr;
>
> @@ -30,6 +39,8 @@ static struct { char _entry[32]; } hypercall_page[128]
>__attribute__ ((aligned (GRUB_XEN_PAGE_SIZE)));
>
>  static grub_uint32_t xen_cpuid_base;
> +static struct xen_machine_mmap_entry map[128];

A constant instead of 128? If no why 128?

> +static unsigned int nr_map_entries;
>
>  static void
>  grub_xen_early_halt (void)
> @@ -93,11 +104,98 @@ grub_xen_hypercall (grub_uint32_t callno, grub_uint32_t 
> a0,
>return __res;
>  }
>
> +static void
> +grub_xen_sort_mmap (void)
> +{
> +  grub_uint64_t from, to;
> +  unsigned int i;
> +  struct xen_machine_mmap_entry tmp;
> +
> +  /* Align map entries to page boundaries. */
> +  for (i = 0; i < nr_map_entries; i++)
> +{
> +  from = map[i].addr;
> +  to = from + map[i].len;
> +  if (map[i].type == GRUB_MEMORY_AVAILABLE)
> + {
> +   from = ALIGN_UP(from, GRUB_XEN_PAGE_SIZE);

Lack of space between macro name and "(".
Here and below...

> +   to = ALIGN_DOWN(to, GRUB_XEN_PAGE_SIZE);
> + }
> +  else
> + {
> +   from = ALIGN_DOWN(from, GRUB_XEN_PAGE_SIZE);
> +   to = ALIGN_UP(to, GRUB_XEN_PAGE_SIZE);
> + }
> +  map[i].addr = from;
> +  map[i].len = to - from;
> +}

[...]

Daniel

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

Re: [Xen-devel] [PATCH v2 10/18] xen: setup hypercall page for PVH

[Daniel Kiper]

On Tue, Oct 09, 2018 at 01:03:09PM +0200, Juergen Gross wrote:
> Add the needed code to setup the hypercall page for calling into the
> Xen hypervisor.
>
> Signed-off-by: Juergen Gross 
> ---
>  grub-core/kern/i386/xen/pvh.c | 70 
> +++
>  1 file changed, 70 insertions(+)
>
> diff --git a/grub-core/kern/i386/xen/pvh.c b/grub-core/kern/i386/xen/pvh.c
> index 182ef95f9..c1b1cf8db 100644
> --- a/grub-core/kern/i386/xen/pvh.c
> +++ b/grub-core/kern/i386/xen/pvh.c
> @@ -20,14 +20,84 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>
>  grub_uint64_t grub_rsdp_addr;
>
> +static struct { char _entry[32]; } hypercall_page[128]
> +  __attribute__ ((aligned (GRUB_XEN_PAGE_SIZE)));
> +
> +static grub_uint32_t xen_cpuid_base;
> +
> +static void
> +grub_xen_early_halt (void)
> +{
> +  asm volatile ("hlt");
> +}
> +
> +static void
> +grub_xen_cpuid_base (void)
> +{
> +  grub_uint32_t base, eax, signature[3];
> +
> +  for (base = 0x4000; base < 0x4001; base += 0x100)
> +{
> +  grub_cpuid (base, eax, signature[0], signature[1], signature[2]);
> +  if (!grub_memcmp ("XenVMMXenVMM", signature, 12) && (eax - base) >= 2)
> + {
> +   xen_cpuid_base = base;
> +   return;
> + }
> +}
> +
> +  grub_xen_early_halt ();
> +}
> +
> +static void
> +grub_xen_setup_hypercall_page (void)
> +{
> +  grub_uint32_t msr, pfn, eax, ebx, ecx, edx;
> +
> +  grub_cpuid (xen_cpuid_base + 2, eax, ebx, ecx, edx);
> +  msr = ebx;
> +  pfn = (grub_uint32_t) (_page[0]);
> +
> +  asm volatile ("wrmsr" : : "c" (msr), "a" (pfn), "d" (0) : "memory");
> +}
> +
> +int
> +grub_xen_hypercall (grub_uint32_t callno, grub_uint32_t a0,
> + grub_uint32_t a1, grub_uint32_t a2,
> + grub_uint32_t a3, grub_uint32_t a4,
> + grub_uint32_t a5 __attribute__ ((unused)))
> +{
> +  register unsigned long __res  asm("eax");
> +  register unsigned long __arg0 asm("ebx") = __arg0;
> +  register unsigned long __arg1 asm("ecx") = __arg1;
> +  register unsigned long __arg2 asm("edx") = __arg2;
> +  register unsigned long __arg3 asm("esi") = __arg3;
> +  register unsigned long __arg4 asm("edi") = __arg4;

Why do we need this play with registers. Does not asm below
work with "a", "b", ... like above?

> +
> +  __arg0 = a0;
> +  __arg1 = a1;
> +  __arg2 = a2;
> +  __arg3 = a3;
> +  __arg4 = a4;
> +  asm volatile ("call *%[callno]"
> + : "=r" (__res), "+r" (__arg0), "+r" (__arg1), "+r" (__arg2),
> +   "+r" (__arg3), "+r" (__arg4)
> + : [callno] "a" (_page[callno])
> + : "memory");

Daniel

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

Re: [Xen-devel] [PATCH v2 09/18] xen: add PVH boot entry code

[Daniel Kiper]

On Tue, Oct 09, 2018 at 01:03:08PM +0200, Juergen Gross wrote:
> Add the code for the Xen PVH mode boot entry.
>
> Signed-off-by: Juergen Gross 
> ---
>  grub-core/kern/i386/xen/startup_pvh.S | 50 
> +++
>  1 file changed, 50 insertions(+)
>
> diff --git a/grub-core/kern/i386/xen/startup_pvh.S 
> b/grub-core/kern/i386/xen/startup_pvh.S
> index e18ee5b31..0ddb63b31 100644
> --- a/grub-core/kern/i386/xen/startup_pvh.S
> +++ b/grub-core/kern/i386/xen/startup_pvh.S
> @@ -19,11 +19,61 @@
>
>  #include 
>  #include 
> +#include 
>
>   .file   "startup_pvh.S"
>   .text
> + .globl  start, _start
> + .code32
>
> +start:
> +_start:
> + cld
> + lgdtgdtdesc
> + ljmp$GRUB_MEMORY_MACHINE_PROT_MODE_CSEG, $1f
> +1:
> + movl$GRUB_MEMORY_MACHINE_PROT_MODE_DSEG, %eax
> + mov %eax, %ds
> + mov %eax, %es
> + mov %eax, %ss

Should not you load null descriptor into %fs and %gs?
Just in case...

> + lealLOCAL(stack_end), %esp
> +
> + /* Save address of start info structure. */
> + mov %ebx, pvh_start_info
> + callEXT_C(grub_main)
> + /* Doesn't return. */
> +
> + .p2align3
> +gdt:
> + .word   0, 0
> + .byte   0, 0, 0, 0
> +
> + /* -- code segment --
> +  * base = 0x, limit = 0xF (4 KiB Granularity), present
> +  * type = 32bit code execute/read, DPL = 0
> +  */
> + .word   0x, 0
> + .byte   0, 0x9A, 0xCF, 0
> +
> + /* -- data segment --
> +  * base = 0x, limit 0xF (4 KiB Granularity), present
> +  * type = 32 bit data read/write, DPL = 0
> +  */
> + .word   0x, 0
> + .byte   0, 0x92, 0xCF, 0
> +
> + .p2align3
> +/* this is the GDT descriptor */
> +gdtdesc:
> + .word   0x17/* limit */
> + .long   gdt /* addr */
> +
> + .p2align2
>  /* Saved pointer to start info structure. */
>   .globl  pvh_start_info
>  pvh_start_info:
>   .long   0
> +
> + .bss
> + .space  (1 << 22)

Hmmm... Why do we need 4 MiB here? If this is really needed then it begs for
a comment. And I would like to see a constant instead of plain number here.

Daniel

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

Re: [Xen-devel] [PATCH v2 08/18] xen: add basic hooks for PVH in current code

[Daniel Kiper]

On Tue, Oct 09, 2018 at 01:03:07PM +0200, Juergen Gross wrote:
> Add the hooks to current code needed for Xen PVH.

I am not against the code itself but it would be nice to know why you
add, AIUI, just stubs here which will be filled with proper code later.

Daniel

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

Re: [Xen-devel] [PATCH v2 07/18] xen: add PVH specific defines to offset.h

[Daniel Kiper]

On Tue, Oct 09, 2018 at 01:03:06PM +0200, Juergen Gross wrote:
> include/grub/offsets.h needs some defines for Xen PVH mode.
>
> Add them. While at it line up the values in the surrounding lines to
> start at the same column.
>
> Signed-off-by: Juergen Gross 
> ---
>  include/grub/offsets.h | 21 -
>  1 file changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/include/grub/offsets.h b/include/grub/offsets.h
> index 330e4c707..b16353163 100644
> --- a/include/grub/offsets.h
> +++ b/include/grub/offsets.h
> @@ -36,9 +36,10 @@
>  #define GRUB_DECOMPRESSOR_I386_PC_MAX_DECOMPRESSOR_SIZE (0x9000-0x8200)
>
>  /* The segment where the kernel is loaded.  */
> -#define GRUB_BOOT_I386_PC_KERNEL_SEG 0x800
> +#define GRUB_BOOT_I386_PC_KERNEL_SEG 0x800
>
> -#define GRUB_KERNEL_I386_PC_LINK_ADDR  0x9000
> +#define GRUB_KERNEL_I386_PC_LINK_ADDR0x9000
> +#define GRUB_KERNEL_I386_XENPVH_LINK_ADDR0x10

s/XENPVH/XEN_PVH/ In general I prefer XEN_PVH instead of XENPVH.
So, please update them all where possible. Not only in this patch.

>  /* The upper memory area (starting at 640 kiB).  */
>  #define GRUB_MEMORY_I386_PC_UPPER0xa
> @@ -101,15 +102,17 @@
>  #define GRUB_KERNEL_I386_MULTIBOOT_MOD_ALIGN 
> GRUB_KERNEL_I386_COREBOOT_MOD_ALIGN
>
>  #define GRUB_KERNEL_X86_64_XEN_MOD_ALIGN 0x8
> -#define GRUB_KERNEL_I386_XEN_MOD_ALIGN   0x8
> +#define GRUB_KERNEL_I386_XEN_MOD_ALIGN   0x8
> +#define GRUB_KERNEL_I386_XENPVH_MOD_ALIGN0x8

Ditto.

>  /* Non-zero value is only needed for PowerMacs.  */
> -#define GRUB_KERNEL_X86_64_XEN_MOD_GAP 0x0
> -#define GRUB_KERNEL_I386_XEN_MOD_GAP 0x0
> -#define GRUB_KERNEL_I386_IEEE1275_MOD_GAP 0x0
> -#define GRUB_KERNEL_I386_COREBOOT_MOD_GAP 0x0
> -#define GRUB_KERNEL_SPARC64_IEEE1275_MOD_GAP 0x0
> -#define GRUB_KERNEL_ARM_UBOOT_MOD_GAP 0x0
> +#define GRUB_KERNEL_X86_64_XEN_MOD_GAP   0x0
> +#define GRUB_KERNEL_I386_XEN_MOD_GAP 0x0
> +#define GRUB_KERNEL_I386_XENPVH_MOD_GAP  0x0

Ditto.

Daniel

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

Re: [Xen-devel] [PATCH v2 05/18] xen: add some dummy headers for PVH mode

[Daniel Kiper]

On Tue, Oct 09, 2018 at 01:03:04PM +0200, Juergen Gross wrote:
> Xen PVH mode needs some headers including the common i386 headers.
> Add those to the tree.
>
> Signed-off-by: Juergen Gross 
> ---
>  include/grub/i386/xenpvh/boot.h| 1 +
>  include/grub/i386/xenpvh/console.h | 1 +
>  include/grub/i386/xenpvh/int.h | 1 +
>  include/grub/i386/xenpvh/memory.h  | 1 +
>  include/grub/i386/xenpvh/time.h| 1 +
>  5 files changed, 5 insertions(+)
>  create mode 100644 include/grub/i386/xenpvh/boot.h
>  create mode 100644 include/grub/i386/xenpvh/console.h
>  create mode 100644 include/grub/i386/xenpvh/int.h
>  create mode 100644 include/grub/i386/xenpvh/memory.h
>  create mode 100644 include/grub/i386/xenpvh/time.h
>
> diff --git a/include/grub/i386/xenpvh/boot.h b/include/grub/i386/xenpvh/boot.h
> new file mode 100644
> index 0..6cd23aa83
> --- /dev/null
> +++ b/include/grub/i386/xenpvh/boot.h
> @@ -0,0 +1 @@
> +#include 
> diff --git a/include/grub/i386/xenpvh/console.h 
> b/include/grub/i386/xenpvh/console.h
> new file mode 100644
> index 0..305a46d8e
> --- /dev/null
> +++ b/include/grub/i386/xenpvh/console.h
> @@ -0,0 +1 @@
> +#include 
> diff --git a/include/grub/i386/xenpvh/int.h b/include/grub/i386/xenpvh/int.h
> new file mode 100644
> index 0..6f9d14a81
> --- /dev/null
> +++ b/include/grub/i386/xenpvh/int.h
> @@ -0,0 +1 @@
> +#include 
> diff --git a/include/grub/i386/xenpvh/memory.h 
> b/include/grub/i386/xenpvh/memory.h
> new file mode 100644
> index 0..8dd6f7c8c
> --- /dev/null
> +++ b/include/grub/i386/xenpvh/memory.h
> @@ -0,0 +1 @@
> +#include 
> diff --git a/include/grub/i386/xenpvh/time.h b/include/grub/i386/xenpvh/time.h
> new file mode 100644
> index 0..2298ee8f4
> --- /dev/null
> +++ b/include/grub/i386/xenpvh/time.h
> @@ -0,0 +1 @@
> +#include 

I do not understand why we need these files. And commit message just states
just that we need them. However, it says nothing why... So, why? Who will
use them?

Daniel

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

Re: [Xen-devel] [PATCH v2 02/18] loader/linux: support passing rsdp address via boot params

[Daniel Kiper]

On Thu, Oct 18, 2018 at 04:53:01PM +0200, Juergen Gross wrote:
> On 18/10/2018 16:48, Daniel Kiper wrote:
> > On Thu, Oct 18, 2018 at 04:36:28PM +0200, Juergen Gross wrote:
> >> On 18/10/2018 16:30, Daniel Kiper wrote:
> >>> On Thu, Oct 18, 2018 at 04:18:26PM +0200, Juergen Gross wrote:
> >>>> On 18/10/2018 16:13, Daniel Kiper wrote:
> >>>>> On Tue, Oct 09, 2018 at 01:03:01PM +0200, Juergen Gross wrote:
> >>>>>> Xen PVH guests will have the RSDP at an arbitrary address. Support that
> >>>>>> by passing the RSDP address via the boot parameters to Linux.
> >>>>>>
> >>>>>> The new protocol version 2.14 requires to set version to 0x8000 ored
> >>>>>> with the actually use protocol version (the minimum of the kernel
> >>>>>> supplied protocol version and the grub2 supported protocol version)
> >>>>>> if 2.14 or higher are in effect.
> >>>>>>
> >>>>>> Signed-off-by: Juergen Gross 
> >>>>>> ---
> >>>>>> V2: add oring 0x8000 to version field
> >>>>>> ---
> >>>>>>  grub-core/loader/i386/linux.c | 9 +
> >>>>>>  include/grub/i386/linux.h | 5 -
> >>>>>>  2 files changed, 13 insertions(+), 1 deletion(-)
> >>>>>>
> >>>>>> diff --git a/grub-core/loader/i386/linux.c 
> >>>>>> b/grub-core/loader/i386/linux.c
> >>>>>> index 4eab55a2d..f96309476 100644
> >>>>>> --- a/grub-core/loader/i386/linux.c
> >>>>>> +++ b/grub-core/loader/i386/linux.c
> >>>>>> @@ -35,6 +35,7 @@
> >>>>>>  #include 
> >>>>>>  #include 
> >>>>>>  #include 
> >>>>>> +#include 
> >>>>>
> >>>>> Probably this change belongs to another patch.
> >>>>
> >>>> I don't think so.
> >>>
> >>> You do not add anything to this header here and out of the blue you
> >>> include it in this file. So, why it is needed here?
> >>
> >> Ah, now I see your problem.
> >>
> >> machine/kernel.h will be the header which eventually defines
> >> GRUB_KERNEL_USE_RSDP_ADDR.
> >
> > So, please move this to the proper patch.
>
> Okay, if you like that better.

Yes, I will. Thanks!

Daniel

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