[v3,0/5] pvh: add new PVH option rom

Message

Stefano Garzarella Jan. 16, 2019, 5:49 p.m. UTC

This patch series is based on "[PATCH v3 0/4] QEMU changes to do PVH boot" and
provides a PVH option rom that can be used with SeaBIOS to boot uncompressed
kernel using the x86/HVM direct boot ABI.

Patches 1 and 2 are to prepare the PVH option rom, moving common functions in
the new headers.  Patch 3 adds the new PVH option rom. Patch 4 uses it when we
are booting an uncompressed kernel using the x86/HVM direct boot ABI. Patch 5
adds the support of loading the initrd in the PVH option rom.

Based-on: <[e-mail]>

Changes in v3:
- added Patch 5 to support initrd
- addressed comments by Eric and Liam:
    - Patch 3: comments by Liam
    - Patch 4: moved Based-on tag in the cover letter (by Eric)
- Patch 3: added pc-bios/pvh.bin binary in the commit

Changes in v2:
- addressed comments by Stefan and Eric:
  - Patch 2: moved inludes on top of linuxboot_dma.c and add <stdint.h> in
    optrom.h
  - Patch 4: added check of pvh.bin in xen_load_linux()
- modified commit message of patch 2 to explain better the patch

Stefano Garzarella (5):
  linuxboot_dma: remove duplicate definitions of FW_CFG
  linuxboot_dma: move common functions in a new header
  optionrom: add new PVH option rom
  hw/i386/pc: use PVH option rom
  optionrom/pvh: load initrd from fw_cfg

 Makefile                          |   2 +-
 hw/i386/pc.c                      |   5 +
 pc-bios/optionrom/Makefile        |   5 +-
 pc-bios/optionrom/linuxboot_dma.c | 112 +++--------------
 pc-bios/optionrom/optrom.h        | 110 ++++++++++++++++
 pc-bios/optionrom/optrom_fw_cfg.h |  92 ++++++++++++++
 pc-bios/optionrom/pvh.S           | 200 ++++++++++++++++++++++++++++++
 pc-bios/optionrom/pvh_main.c      | 133 ++++++++++++++++++++
 pc-bios/pvh.bin                   | Bin 0 -> 1536 bytes
 9 files changed, 561 insertions(+), 98 deletions(-)
 create mode 100644 pc-bios/optionrom/optrom.h
 create mode 100644 pc-bios/optionrom/optrom_fw_cfg.h
 create mode 100644 pc-bios/optionrom/pvh.S
 create mode 100644 pc-bios/optionrom/pvh_main.c
 create mode 100644 pc-bios/pvh.bin

Comments

Paolo Bonzini Jan. 16, 2019, 6:31 p.m. UTC | #1

On 16/01/19 18:49, Stefano Garzarella wrote:
> This patch series is based on "[PATCH v3 0/4] QEMU changes to do PVH boot" and
> provides a PVH option rom that can be used with SeaBIOS to boot uncompressed
> kernel using the x86/HVM direct boot ABI.
> 
> Patches 1 and 2 are to prepare the PVH option rom, moving common functions in
> the new headers.  Patch 3 adds the new PVH option rom. Patch 4 uses it when we
> are booting an uncompressed kernel using the x86/HVM direct boot ABI. Patch 5
> adds the support of loading the initrd in the PVH option rom.
> 
> Based-on: <[e-mail]>

I get this:

  CC      optionrom/pvh_main.o
/tmp/ccoel69H.s: Assembler messages:
/tmp/ccoel69H.s:171: Error: attempt to move .org backwards

when applying this series on CentOS 7.

Paolo

Paolo Bonzini Jan. 16, 2019, 9:17 p.m. UTC | #2

On 16/01/19 19:31, Paolo Bonzini wrote:
> On 16/01/19 18:49, Stefano Garzarella wrote:
>> This patch series is based on "[PATCH v3 0/4] QEMU changes to do PVH boot" and
>> provides a PVH option rom that can be used with SeaBIOS to boot uncompressed
>> kernel using the x86/HVM direct boot ABI.
>>
>> Patches 1 and 2 are to prepare the PVH option rom, moving common functions in
>> the new headers.  Patch 3 adds the new PVH option rom. Patch 4 uses it when we
>> are booting an uncompressed kernel using the x86/HVM direct boot ABI. Patch 5
>> adds the support of loading the initrd in the PVH option rom.
>>
>> Based-on: <[e-mail]>
> 
> I get this:
> 
>   CC      optionrom/pvh_main.o
> /tmp/ccoel69H.s: Assembler messages:
> /tmp/ccoel69H.s:171: Error: attempt to move .org backwards
> 
> when applying this series on CentOS 7.

Oops, the issue was actaually in linux_dma - I was confused by make -j
reordering the output.  The fix is trivial

--- a/pc-bios/optionrom/linuxboot_dma.c
+++ b/pc-bios/optionrom/linuxboot_dma.c
@@ -20,10 +20,6 @@
  *     Richard W.M. Jones <[e-mail]>
  */

-#include <stdint.h>
-#include "optrom.h"
-#include "optrom_fw_cfg.h"
-
 asm(
 ".text\n"
 ".global _start\n"
@@ -62,6 +58,10 @@ asm(
 "   jmp load_kernel\n"
 );

+#include <stdint.h>
+#include "optrom.h"
+#include "optrom_fw_cfg.h"
+
 static inline void set_es(void *addr)
 {
     uint32_t seg = (uint32_t)addr >> 4;


Also, the $? in the new rule should be $^.  I wouldn't use a pattern
rule for that, writing it explicitly as "pvh.img: pvh.S pvh_main.c"

Thanks,

Paolo

Stefano Garzarella Jan. 17, 2019, 7:50 a.m. UTC | #3

Hi Paolo,

On Wed, Jan 16, 2019 at 10:17:15PM +0100, Paolo Bonzini wrote:
> On 16/01/19 19:31, Paolo Bonzini wrote:
> > 
> > I get this:
> > 
> >   CC      optionrom/pvh_main.o
> > /tmp/ccoel69H.s: Assembler messages:
> > /tmp/ccoel69H.s:171: Error: attempt to move .org backwards
> > 
> > when applying this series on CentOS 7.
> 
> Oops, the issue was actaually in linux_dma - I was confused by make -j
> reordering the output.  The fix is trivial

Thanks for the fix! I didn't have this error on Fedora 29 (gcc 8.2.1).

> 
> --- a/pc-bios/optionrom/linuxboot_dma.c
> +++ b/pc-bios/optionrom/linuxboot_dma.c
> @@ -20,10 +20,6 @@
>   *     Richard W.M. Jones <[e-mail]>
>   */
> 
> -#include <stdint.h>
> -#include "optrom.h"
> -#include "optrom_fw_cfg.h"
> -
>  asm(
>  ".text\n"
>  ".global _start\n"
> @@ -62,6 +58,10 @@ asm(
>  "   jmp load_kernel\n"
>  );
> 
> +#include <stdint.h>
> +#include "optrom.h"
> +#include "optrom_fw_cfg.h"
> +
>  static inline void set_es(void *addr)
>  {
>      uint32_t seg = (uint32_t)addr >> 4;
> 
> 
> Also, the $? in the new rule should be $^.  I wouldn't use a pattern
> rule for that, writing it explicitly as "pvh.img: pvh.S pvh_main.c"

I'll fix the Makefile.

Thanks,
Stefano

> 
> Thanks,
> 
> Paolo

Paolo Bonzini Jan. 17, 2019, 10:31 a.m. UTC | #4

On 17/01/19 08:50, Stefano Garzarella wrote:
>> Also, the $? in the new rule should be $^.  I wouldn't use a pattern
>> rule for that, writing it explicitly as "pvh.img: pvh.S pvh_main.c"
> I'll fix the Makefile.

(of course it's "pvh.img: pvh.o pvh_main.o").

I can fix up as well if it's okay for you.

Paolo

Stefano Garzarella Jan. 17, 2019, 10:36 a.m. UTC | #5

On Thu, Jan 17, 2019 at 11:31:27AM +0100, Paolo Bonzini wrote:
> On 17/01/19 08:50, Stefano Garzarella wrote:
> >> Also, the $? in the new rule should be $^.  I wouldn't use a pattern
> >> rule for that, writing it explicitly as "pvh.img: pvh.S pvh_main.c"
> > I'll fix the Makefile.
> 
> (of course it's "pvh.img: pvh.o pvh_main.o").
> 
> I can fix up as well if it's okay for you.

I already sent a v4 fixing your comments (and I put "pvh.img: pvh.o
pvh_main.o" :) ), but if want to fix this series is okay for me.

Thanks,
Stefano