| Message ID | 20190115100058.44712-5-sgarzare@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | pvh: add new PVH option rom | expand |
On Tue, Jan 15, 2019 at 11:00:58AM +0100, Stefano Garzarella wrote: > Use pvh.bin option rom when we are booting an uncompressed > kernel using the x86/HVM direct boot ABI. > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > Based-on: <1545422632-24444-5-git-send-email-liam.merwick@oracle.com> I don't think this is a great way to give attribution. Can you pls include the author name and the S.O.B from there as well? > --- > hw/i386/pc.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index 06bce6a101..7564ba51d2 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -1005,6 +1005,10 @@ static void load_linux(PCMachineState *pcms, > fw_cfg_add_bytes(fw_cfg, FW_CFG_SETUP_DATA, > header, sizeof(header)); > > + option_rom[nb_option_roms].bootindex = 0; > + option_rom[nb_option_roms].name = "pvh.bin"; > + nb_option_roms++; > + > return; > } > /* This looks like a multiboot kernel. If it is, let's stop > @@ -1456,6 +1460,7 @@ void xen_load_linux(PCMachineState *pcms) > for (i = 0; i < nb_option_roms; i++) { > assert(!strcmp(option_rom[i].name, "linuxboot.bin") || > !strcmp(option_rom[i].name, "linuxboot_dma.bin") || > + !strcmp(option_rom[i].name, "pvh.bin") || > !strcmp(option_rom[i].name, "multiboot.bin")); > rom_add_option(option_rom[i].name, option_rom[i].bootindex); > } OK but this is guest visible so needs to be guarded by the new machine type. > -- > 2.20.1
On 1/15/19 12:57 PM, Michael S. Tsirkin wrote: > On Tue, Jan 15, 2019 at 11:00:58AM +0100, Stefano Garzarella wrote: >> Use pvh.bin option rom when we are booting an uncompressed >> kernel using the x86/HVM direct boot ABI. >> >> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> >> Based-on: <1545422632-24444-5-git-send-email-liam.merwick@oracle.com> > > I don't think this is a great way to give attribution. > Can you pls include the author name and the S.O.B from there as well? > > >> --- Patchew understands Based-on: tags as meaning a prerequisite patch on the list that must be applied first before this patch can apply (and NOT in the sense that this patch is an updated revision to replace an earlier revision posted by someone else) - but when using the annotation to keep Patchew happy, it should either be in a 0/N cover letter, or for a single patch, after the --- separator, as the condition of a prerequisite patch not being applied is a transient condition relevant to the current build but not something that needs to be recorded in long-term git history.
On Tue, Jan 15, 2019 at 01:12:21PM -0600, Eric Blake wrote: > On 1/15/19 12:57 PM, Michael S. Tsirkin wrote: > > On Tue, Jan 15, 2019 at 11:00:58AM +0100, Stefano Garzarella wrote: > >> Use pvh.bin option rom when we are booting an uncompressed > >> kernel using the x86/HVM direct boot ABI. > >> > >> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > >> Based-on: <1545422632-24444-5-git-send-email-liam.merwick@oracle.com> > > > > I don't think this is a great way to give attribution. > > Can you pls include the author name and the S.O.B from there as well? > > > > > >> --- > > Patchew understands Based-on: tags as meaning a prerequisite patch on > the list that must be applied first before this patch can apply (and NOT > in the sense that this patch is an updated revision to replace an > earlier revision posted by someone else) - but when using the annotation > to keep Patchew happy, it should either be in a 0/N cover letter, or for > a single patch, after the --- separator, as the condition of a > prerequisite patch not being applied is a transient condition relevant > to the current build but not something that needs to be recorded in > long-term git history. > > -- > Eric Blake, Principal Software Engineer > Red Hat, Inc. +1-919-301-3226 > Virtualization: qemu.org | libvirt.org > Good to know. But I still think listing the original author (e.g. Based on patches by Foo Bar) or such is the nice thing to do.
On 15/01/19 21:05, Michael S. Tsirkin wrote: > On Tue, Jan 15, 2019 at 01:12:21PM -0600, Eric Blake wrote: >> On 1/15/19 12:57 PM, Michael S. Tsirkin wrote: >>> On Tue, Jan 15, 2019 at 11:00:58AM +0100, Stefano Garzarella wrote: >>>> Use pvh.bin option rom when we are booting an uncompressed >>>> kernel using the x86/HVM direct boot ABI. >>>> >>>> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> >>>> Based-on: <1545422632-24444-5-git-send-email-liam.merwick@oracle.com> >>> >>> I don't think this is a great way to give attribution. >>> Can you pls include the author name and the S.O.B from there as well? > > Good to know. > But I still think listing the original author (e.g. Based on patches by > Foo Bar) or such is the nice thing to do. It's not based on patches from another author, the work is entirely Stefano's. It must be applied on top of those patches. Paolo
Hi Michael, On Tue, Jan 15, 2019 at 03:05:27PM -0500, Michael S. Tsirkin wrote: > On Tue, Jan 15, 2019 at 01:12:21PM -0600, Eric Blake wrote: > > On 1/15/19 12:57 PM, Michael S. Tsirkin wrote: > > > On Tue, Jan 15, 2019 at 11:00:58AM +0100, Stefano Garzarella wrote: > > >> Use pvh.bin option rom when we are booting an uncompressed > > >> kernel using the x86/HVM direct boot ABI. > > >> > > >> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > > >> Based-on: <1545422632-24444-5-git-send-email-liam.merwick@oracle.com> > > > > > > I don't think this is a great way to give attribution. > > > Can you pls include the author name and the S.O.B from there as well? > > > > > > > > >> --- > > > > Patchew understands Based-on: tags as meaning a prerequisite patch on > > the list that must be applied first before this patch can apply (and NOT > > in the sense that this patch is an updated revision to replace an > > earlier revision posted by someone else) - but when using the annotation > > to keep Patchew happy, it should either be in a 0/N cover letter, or for > > a single patch, after the --- separator, as the condition of a > > prerequisite patch not being applied is a transient condition relevant > > to the current build but not something that needs to be recorded in > > long-term git history. > > > > -- > > Eric Blake, Principal Software Engineer > > Red Hat, Inc. +1-919-301-3226 > > Virtualization: qemu.org | libvirt.org > > > > Good to know. > But I still think listing the original author (e.g. Based on patches by > Foo Bar) or such is the nice thing to do. very sorry for the misunderstanding, as Eric and Paolo said, I used Based-on tag only to inform Patchew and maintainers that this patch must be applied after Liam's patch that is under review. Thanks, Stefano > > -- > MST
Hi Eric, On Tue, Jan 15, 2019 at 01:12:21PM -0600, Eric Blake wrote: > On 1/15/19 12:57 PM, Michael S. Tsirkin wrote: > > On Tue, Jan 15, 2019 at 11:00:58AM +0100, Stefano Garzarella wrote: > >> Use pvh.bin option rom when we are booting an uncompressed > >> kernel using the x86/HVM direct boot ABI. > >> > >> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > >> Based-on: <1545422632-24444-5-git-send-email-liam.merwick@oracle.com> > > > > I don't think this is a great way to give attribution. > > Can you pls include the author name and the S.O.B from there as well? > > > > > >> --- > > Patchew understands Based-on: tags as meaning a prerequisite patch on > the list that must be applied first before this patch can apply (and NOT > in the sense that this patch is an updated revision to replace an > earlier revision posted by someone else) - but when using the annotation > to keep Patchew happy, it should either be in a 0/N cover letter, or for > a single patch, after the --- separator, as the condition of a > prerequisite patch not being applied is a transient condition relevant > to the current build but not something that needs to be recorded in > long-term git history. > Thank you very much for the explanation! I'll move the Based-on tag in the cover letter. Cheers, Stefano
On Tue, Jan 15, 2019 at 01:57:22PM -0500, Michael S. Tsirkin wrote: > On Tue, Jan 15, 2019 at 11:00:58AM +0100, Stefano Garzarella wrote: > > Use pvh.bin option rom when we are booting an uncompressed > > kernel using the x86/HVM direct boot ABI. > > > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > > Based-on: <1545422632-24444-5-git-send-email-liam.merwick@oracle.com> > > I don't think this is a great way to give attribution. > Can you pls include the author name and the S.O.B from there as well? > > > > --- > > hw/i386/pc.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > > index 06bce6a101..7564ba51d2 100644 > > --- a/hw/i386/pc.c > > +++ b/hw/i386/pc.c > > @@ -1005,6 +1005,10 @@ static void load_linux(PCMachineState *pcms, > > fw_cfg_add_bytes(fw_cfg, FW_CFG_SETUP_DATA, > > header, sizeof(header)); > > > > + option_rom[nb_option_roms].bootindex = 0; > > + option_rom[nb_option_roms].name = "pvh.bin"; > > + nb_option_roms++; > > + > > return; > > } > > /* This looks like a multiboot kernel. If it is, let's stop > > @@ -1456,6 +1460,7 @@ void xen_load_linux(PCMachineState *pcms) > > for (i = 0; i < nb_option_roms; i++) { > > assert(!strcmp(option_rom[i].name, "linuxboot.bin") || > > !strcmp(option_rom[i].name, "linuxboot_dma.bin") || > > + !strcmp(option_rom[i].name, "pvh.bin") || > > !strcmp(option_rom[i].name, "multiboot.bin")); > > rom_add_option(option_rom[i].name, option_rom[i].bootindex); > > } > > OK but this is guest visible so needs to be guarded by the > new machine type. Aren't option ROMs treated like other firmware? i.e.: guest visible, but copied during live migration and not considered part of guest ABI.
diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 06bce6a101..7564ba51d2 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1005,6 +1005,10 @@ static void load_linux(PCMachineState *pcms, fw_cfg_add_bytes(fw_cfg, FW_CFG_SETUP_DATA, header, sizeof(header)); + option_rom[nb_option_roms].bootindex = 0; + option_rom[nb_option_roms].name = "pvh.bin"; + nb_option_roms++; + return; } /* This looks like a multiboot kernel. If it is, let's stop @@ -1456,6 +1460,7 @@ void xen_load_linux(PCMachineState *pcms) for (i = 0; i < nb_option_roms; i++) { assert(!strcmp(option_rom[i].name, "linuxboot.bin") || !strcmp(option_rom[i].name, "linuxboot_dma.bin") || + !strcmp(option_rom[i].name, "pvh.bin") || !strcmp(option_rom[i].name, "multiboot.bin")); rom_add_option(option_rom[i].name, option_rom[i].bootindex); }
Use pvh.bin option rom when we are booting an uncompressed kernel using the x86/HVM direct boot ABI. Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> Based-on: <1545422632-24444-5-git-send-email-liam.merwick@oracle.com> --- hw/i386/pc.c | 5 +++++ 1 file changed, 5 insertions(+)