| Message ID | 20190117090259.32713-6-sgarzare@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | pvh: add new PVH option rom | expand |
On Thu, Jan 17, 2019 at 10:02:59AM +0100, Stefano Garzarella wrote: > If we found initrd through fw_cfg, we can load it and use the > first module of hvm_start_info to pass initrd address and size > to the kernel. > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > --- > pc-bios/optionrom/pvh_main.c | 21 +++++++++++++++++++-- > pc-bios/pvh.bin | Bin 1536 -> 1536 bytes > 2 files changed, 19 insertions(+), 2 deletions(-) Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Hi Stefano, On 17/01/2019 09:02, Stefano Garzarella wrote: > If we found initrd through fw_cfg, we can load it and use the > first module of hvm_start_info to pass initrd address and size > to the kernel. > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > --- > pc-bios/optionrom/pvh_main.c | 21 +++++++++++++++++++-- > pc-bios/pvh.bin | Bin 1536 -> 1536 bytes > 2 files changed, 19 insertions(+), 2 deletions(-) > > diff --git a/pc-bios/optionrom/pvh_main.c b/pc-bios/optionrom/pvh_main.c > index f355476e68..d1b8b4b96e 100644 > --- a/pc-bios/optionrom/pvh_main.c > +++ b/pc-bios/optionrom/pvh_main.c > @@ -46,6 +46,7 @@ struct pvh_e820_table { > struct pvh_e820_table pvh_e820 __attribute__ ((aligned)); > > static struct hvm_start_info start_info; > +static struct hvm_modlist_entry ramdisk_mod; > static uint8_t cmdline_buffer[CMDLINE_BUFSIZE]; > > > @@ -71,8 +72,8 @@ extern void pvh_load_kernel(void) asm("pvh_load_kernel"); > void pvh_load_kernel(void) > { > void *cmdline_addr = &cmdline_buffer; > - void *kernel_entry; > - uint32_t cmdline_size, fw_cfg_version = bios_cfg_version(); > + void *kernel_entry, *initrd_addr; > + uint32_t cmdline_size, initrd_size, fw_cfg_version = bios_cfg_version(); > > start_info.magic = XEN_HVM_START_MAGIC_VALUE; > start_info.version = 1; > @@ -110,6 +111,22 @@ void pvh_load_kernel(void) > fw_cfg_version); > start_info.cmdline_paddr = (uintptr_t)cmdline_addr; > > + /* Check if we have the initrd to load */ > + bios_cfg_read_entry(&initrd_size, FW_CFG_INITRD_SIZE, 4, fw_cfg_version); > + if (initrd_size) { > + bios_cfg_read_entry(&initrd_addr, FW_CFG_INITRD_ADDR, 4, > + fw_cfg_version); > + bios_cfg_read_entry(initrd_addr, FW_CFG_INITRD_DATA, initrd_size, > + fw_cfg_version); > + > + ramdisk_mod.paddr = (uintptr_t)initrd_addr; > + ramdisk_mod.size = initrd_size; > + > + /* The first module is always ramdisk. */ > + start_info.modlist_paddr = (uintptr_t)&ramdisk_mod; > + start_info.nr_modules = 1; > + } > + > bios_cfg_read_entry(&kernel_entry, FW_CFG_KERNEL_ENTRY, 4, fw_cfg_version); > > asm volatile("jmp *%1" : : "b"(&start_info), "c"(kernel_entry)); > diff --git a/pc-bios/pvh.bin b/pc-bios/pvh.bin I'm not sure what this binary is doing here but it reminded me that the following entries should be added to .gitignore in one of the patches. # pc-bios/optionrom/pvh.bin # pc-bios/optionrom/pvh.img # pc-bios/optionrom/pvh.raw other than that, the code here LGTM so for that Reviewed-by: Liam Merwick <liam.merwick@oracle.com> > index 38a41761014957d50eb55d790b6957888cbeee0a..8033080ada2db4c4613fdc3bb5a69d79c7b0c0ca 100644 > GIT binary patch > delta 735 > zcmZqRY2cYKndyM&#;Nv<`Y|dZ$5|Uh85k;#<{2MnZ2__v85kHA9sn{HZvYZIPJmb| > zfW&duHKLQ5nUrMIfhw902-q@k@G+F$J<hrSC<y|`S(k`Tc4U&S-@w4az+ic-T<m2U > z69Yr<HjpGxgXJ;)wosrzw?N~kpkNn;ZV#5mj{zY;3Y-^99~fR*3^J!vbP15`Eo0c1 > zzzXD<s2q1u0U8F>|Cout)f{N^UIC!Q4iTVG=c}~$y9I%~ogfvxH7Wv~w?60W5Uq#G > zb^8ECzJtWNSyZ}1R9F_z0NXGJNbcZ<DuU@10jqOS5otY88nQzGD7y<}QQCW$0*+lE > zt3K!K289;TXpa96n~%su$Hm7U?k!Pcm;iP@C<qup_A@ZNwE6%4fA1DG5F4yu2N#e9 > zax=_C9<XzTprSw{MY?NLz=GW_Dm;r5fc6|`O#zZ102C5goB<ZffeP&asaQM#EH(uy > zcAT|GlmilnFD)iJGRxEpyomk#|34@!c7fD%et6*v;$wse*j2kg!O{z~>V*bKQ3x<N > zE|p|Lq(IIGO7Xnlg_tL@3*>Z=Q(yf415}LN%^5^G8scV%O`y;R+Qji<3DDe^<^K@j > Z2no&!h6hlR%ga4L7i~VsEX~MR4FFIY;aUIy > > delta 434 > zcmZqRY2cYKnW;f!<5YV_{Sp<C<E#!M3=9=V^Nf$Px&T>>3=9kl8-R?(1wdj)2Z)sc > zB#yJ@h)iZ?Qj%c@s%SnSV9UV4$549rIBNn>5(JL3rie^-WRk9*!N9`6V0pM)?4=n{ > zY40|WBv6CpVg5E%pg^}k<ENlt7lm#Qmd1|(Awde97fT-)URn$?r&BZqNcNU7>~;W} > z(J7;H+(iY*0P26n#NRp{$k;2u3Un37%Fb75@AnA;c{@QWdTUffIzN5R*&&(+mFxBa > zirfc@b+f2+hp4bDjsV*b10;6{Ko!CC3V_t@289Gro525v%|~RS<Kklv_ij-Gs$@u; > z0Cpb8iy$ro!^@b-51D1cMPB^<`~N@4rd^^y(asw$-h%iT_JOV01+ui)MMdDn3812v > j<^TWx2l~|T0E*vVzWMk6|K^V@e2kM=|4(AIW9$F`FOZ8_ >
On 17/01/19 15:33, Liam Merwick wrote: > # pc-bios/optionrom/pvh.bin > # pc-bios/optionrom/pvh.img > # pc-bios/optionrom/pvh.raw pvh.bin should not be ignored. Paolo
On 17/01/2019 14:37, Paolo Bonzini wrote: > On 17/01/19 15:33, Liam Merwick wrote: >> # pc-bios/optionrom/pvh.bin >> # pc-bios/optionrom/pvh.img >> # pc-bios/optionrom/pvh.raw > > pvh.bin should not be ignored. > That's part of what I didn't quite understand. pc-bios/optionrom/linuxboot.bin (and the other binaries in the 'build-all' target in pc-bios/optionrom/Makefile) are in .gitignore. Also, should pvh.bin be added to BLOBS in Makefile like those files? Regards, Liam
On Thu, Jan 17, 2019 at 03:37:24PM +0100, Paolo Bonzini wrote: > On 17/01/19 15:33, Liam Merwick wrote: > > # pc-bios/optionrom/pvh.bin > > # pc-bios/optionrom/pvh.img > > # pc-bios/optionrom/pvh.raw Thanks Liam! > > pvh.bin should not be ignored. I committed pc-bios/pvh.bin, but I think we should ignore the pc-bios/optionrom/pvh.bin like for the other optionroms (eg. pc-bios/optionrom/linuxboot_dma.bin is present in .gitignore) What do you think if I add the following at patch 3/5? diff --git a/.gitignore b/.gitignore index 0430257313..321095bf1a 100644 --- a/.gitignore +++ b/.gitignore @@ -103,6 +103,10 @@ /pc-bios/optionrom/linuxboot_dma.bin /pc-bios/optionrom/linuxboot_dma.raw /pc-bios/optionrom/linuxboot_dma.img +/pc-bios/optionrom/pvh.asm +/pc-bios/optionrom/pvh.bin +/pc-bios/optionrom/pvh.raw +/pc-bios/optionrom/pvh.img /pc-bios/optionrom/multiboot.asm /pc-bios/optionrom/multiboot.bin /pc-bios/optionrom/multiboot.raw Thanks, Stefano
On Thu, Jan 17, 2019 at 02:52:25PM +0000, Liam Merwick wrote: > > > On 17/01/2019 14:37, Paolo Bonzini wrote: > > On 17/01/19 15:33, Liam Merwick wrote: > > > # pc-bios/optionrom/pvh.bin > > > # pc-bios/optionrom/pvh.img > > > # pc-bios/optionrom/pvh.raw > > > > pvh.bin should not be ignored. > > > > That's part of what I didn't quite understand. > pc-bios/optionrom/linuxboot.bin (and the other binaries in the 'build-all' > target in pc-bios/optionrom/Makefile) are in .gitignore. > > Also, should pvh.bin be added to BLOBS in Makefile like those files? Yes, indeed I added pvh.bin to BLOBS in patch 3/5 and I committed the binary file in /pc-bios/ (note: not in /pc-bios/optionrom/ where it is compiled, maybe is here the confusion, but I following what was done for the other option roms) Of course, I forgot to add the pc-bios/optionrom/pvh.* entries in .gitignore :) Thanks, Stefano
On 17/01/19 16:05, Stefano Garzarella wrote: >> That's part of what I didn't quite understand. >> pc-bios/optionrom/linuxboot.bin (and the other binaries in the 'build-all' >> target in pc-bios/optionrom/Makefile) are in .gitignore. >> >> Also, should pvh.bin be added to BLOBS in Makefile like those files? > Yes, indeed I added pvh.bin to BLOBS in patch 3/5 and I committed the > binary file in /pc-bios/ (note: not in /pc-bios/optionrom/ where it is > compiled, maybe is here the confusion, but I following what was done for > the other option roms) Nevermind---I was totally sure .bin files are committed. Though perhaps they should, since non-x86 binaries are. Paolo
On Thu, Jan 17, 2019 at 04:18:00PM +0100, Paolo Bonzini wrote: > On 17/01/19 16:05, Stefano Garzarella wrote: > >> That's part of what I didn't quite understand. > >> pc-bios/optionrom/linuxboot.bin (and the other binaries in the 'build-all' > >> target in pc-bios/optionrom/Makefile) are in .gitignore. > >> > >> Also, should pvh.bin be added to BLOBS in Makefile like those files? > > Yes, indeed I added pvh.bin to BLOBS in patch 3/5 and I committed the > > binary file in /pc-bios/ (note: not in /pc-bios/optionrom/ where it is > > compiled, maybe is here the confusion, but I following what was done for > > the other option roms) > > Nevermind---I was totally sure .bin files are committed. Though perhaps > they should, since non-x86 binaries are. They are committed (in /pc-bios/) but not were they are compiled (in /pc-bios/optionrom/).
On 17/01/19 17:13, Stefano Garzarella wrote: > On Thu, Jan 17, 2019 at 04:18:00PM +0100, Paolo Bonzini wrote: >> On 17/01/19 16:05, Stefano Garzarella wrote: >>>> That's part of what I didn't quite understand. >>>> pc-bios/optionrom/linuxboot.bin (and the other binaries in the 'build-all' >>>> target in pc-bios/optionrom/Makefile) are in .gitignore. >>>> >>>> Also, should pvh.bin be added to BLOBS in Makefile like those files? >>> Yes, indeed I added pvh.bin to BLOBS in patch 3/5 and I committed the >>> binary file in /pc-bios/ (note: not in /pc-bios/optionrom/ where it is >>> compiled, maybe is here the confusion, but I following what was done for >>> the other option roms) >> >> Nevermind---I was totally sure .bin files are committed. Though perhaps >> they should, since non-x86 binaries are. > > They are committed (in /pc-bios/) but not were they are compiled > (in /pc-bios/optionrom/). Oh, I see. Sorry if I'm so dumb sometimes... Paolo
diff --git a/pc-bios/optionrom/pvh_main.c b/pc-bios/optionrom/pvh_main.c index f355476e68..d1b8b4b96e 100644 --- a/pc-bios/optionrom/pvh_main.c +++ b/pc-bios/optionrom/pvh_main.c @@ -46,6 +46,7 @@ struct pvh_e820_table { struct pvh_e820_table pvh_e820 __attribute__ ((aligned)); static struct hvm_start_info start_info; +static struct hvm_modlist_entry ramdisk_mod; static uint8_t cmdline_buffer[CMDLINE_BUFSIZE]; @@ -71,8 +72,8 @@ extern void pvh_load_kernel(void) asm("pvh_load_kernel"); void pvh_load_kernel(void) { void *cmdline_addr = &cmdline_buffer; - void *kernel_entry; - uint32_t cmdline_size, fw_cfg_version = bios_cfg_version(); + void *kernel_entry, *initrd_addr; + uint32_t cmdline_size, initrd_size, fw_cfg_version = bios_cfg_version(); start_info.magic = XEN_HVM_START_MAGIC_VALUE; start_info.version = 1; @@ -110,6 +111,22 @@ void pvh_load_kernel(void) fw_cfg_version); start_info.cmdline_paddr = (uintptr_t)cmdline_addr; + /* Check if we have the initrd to load */ + bios_cfg_read_entry(&initrd_size, FW_CFG_INITRD_SIZE, 4, fw_cfg_version); + if (initrd_size) { + bios_cfg_read_entry(&initrd_addr, FW_CFG_INITRD_ADDR, 4, + fw_cfg_version); + bios_cfg_read_entry(initrd_addr, FW_CFG_INITRD_DATA, initrd_size, + fw_cfg_version); + + ramdisk_mod.paddr = (uintptr_t)initrd_addr; + ramdisk_mod.size = initrd_size; + + /* The first module is always ramdisk. */ + start_info.modlist_paddr = (uintptr_t)&ramdisk_mod; + start_info.nr_modules = 1; + } + bios_cfg_read_entry(&kernel_entry, FW_CFG_KERNEL_ENTRY, 4, fw_cfg_version); asm volatile("jmp *%1" : : "b"(&start_info), "c"(kernel_entry));
If we found initrd through fw_cfg, we can load it and use the first module of hvm_start_info to pass initrd address and size to the kernel. Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> --- pc-bios/optionrom/pvh_main.c | 21 +++++++++++++++++++-- pc-bios/pvh.bin | Bin 1536 -> 1536 bytes 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/pc-bios/pvh.bin b/pc-bios/pvh.bin index 38a41761014957d50eb55d790b6957888cbeee0a..8033080ada2db4c4613fdc3bb5a69d79c7b0c0ca 100644 GIT binary patch delta 735 zcmZqRY2cYKndyM&#;Nv<`Y|dZ$5|Uh85k;#<{2MnZ2__v85kHA9sn{HZvYZIPJmb| zfW&duHKLQ5nUrMIfhw902-q@k@G+F$J<hrSC<y|`S(k`Tc4U&S-@w4az+ic-T<m2U z69Yr<HjpGxgXJ;)wosrzw?N~kpkNn;ZV#5mj{zY;3Y-^99~fR*3^J!vbP15`Eo0c1 zzzXD<s2q1u0U8F>|Cout)f{N^UIC!Q4iTVG=c}~$y9I%~ogfvxH7Wv~w?60W5Uq#G zb^8ECzJtWNSyZ}1R9F_z0NXGJNbcZ<DuU@10jqOS5otY88nQzGD7y<}QQCW$0*+lE zt3K!K289;TXpa96n~%su$Hm7U?k!Pcm;iP@C<qup_A@ZNwE6%4fA1DG5F4yu2N#e9 zax=_C9<XzTprSw{MY?NLz=GW_Dm;r5fc6|`O#zZ102C5goB<ZffeP&asaQM#EH(uy zcAT|GlmilnFD)iJGRxEpyomk#|34@!c7fD%et6*v;$wse*j2kg!O{z~>V*bKQ3x<N zE|p|Lq(IIGO7Xnlg_tL@3*>Z=Q(yf415}LN%^5^G8scV%O`y;R+Qji<3DDe^<^K@j Z2no&!h6hlR%ga4L7i~VsEX~MR4FFIY;aUIy delta 434 zcmZqRY2cYKnW;f!<5YV_{Sp<C<E#!M3=9=V^Nf$Px&T>>3=9kl8-R?(1wdj)2Z)sc zB#yJ@h)iZ?Qj%c@s%SnSV9UV4$549rIBNn>5(JL3rie^-WRk9*!N9`6V0pM)?4=n{ zY40|WBv6CpVg5E%pg^}k<ENlt7lm#Qmd1|(Awde97fT-)URn$?r&BZqNcNU7>~;W} z(J7;H+(iY*0P26n#NRp{$k;2u3Un37%Fb75@AnA;c{@QWdTUffIzN5R*&&(+mFxBa zirfc@b+f2+hp4bDjsV*b10;6{Ko!CC3V_t@289Gro525v%|~RS<Kklv_ij-Gs$@u; z0Cpb8iy$ro!^@b-51D1cMPB^<`~N@4rd^^y(asw$-h%iT_JOV01+ui)MMdDn3812v j<^TWx2l~|T0E*vVzWMk6|K^V@e2kM=|4(AIW9$F`FOZ8_