On Sat, 25 Feb 2017 07:18:29 +0100
Thomas Huth <th...@redhat.com> wrote:
> On 23.02.2017 13:20, Cornelia Huck wrote:
> > From: Farhan Ali <al...@linux.vnet.ibm.com>
> >
> > Load the network boot image into guest RAM when the boot
> > device selected is a network device. Use some of the reserved
> > space in IplBlockCcw to store the start address of the netboot
> > image.
> >
> > A user could also use 'chreipl'(diag 308/5) to change the boot device.
> > So every time we update the IPLB, we need to verify if the selected
> > boot device is a network device so we can appropriately load the
> > network boot image.
> >
> > Signed-off-by: Farhan Ali <al...@linux.vnet.ibm.com>
> > Signed-off-by: Cornelia Huck <cornelia.h...@de.ibm.com>
> > ---
> > hw/s390x/ipl.c | 89
> > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > hw/s390x/ipl.h | 4 ++-
> > 2 files changed, 92 insertions(+), 1 deletion(-)
> > +static bool is_virtio_net_device(IplParameterBlock *iplb)
> > +{
> > + uint8_t cssid;
> > + uint8_t ssid;
> > + uint16_t devno;
> > + uint16_t schid;
> > + SubchDev *sch = NULL;
> > +
> > + if (iplb->pbt != S390_IPL_TYPE_CCW) {
> > + return false;
> > + }
> > +
> > + devno = be16_to_cpu(iplb->ccw.devno);
> > + ssid = iplb->ccw.ssid & 3;
> > +
> > + for (schid = 0; schid < MAX_SCHID; schid++) {
> > + for (cssid = 0; cssid < MAX_CSSID; cssid++) {
> > + sch = css_find_subch(1, cssid, ssid, schid);
> > +
> > + if (sch && sch->devno == devno) {
> > + return sch->id.cu_model == VIRTIO_ID_NET;
> > + }
> > + }
> > + }
> > + return false;
>
> The above line has only 3 instead of 4 spaces. I wonder why checkpatch
> does not complain here...?
Odd. Will fixup.
>
> > +}
> > + if (ipl->netboot) {
> > + if (load_netboot_image(&err) < 0) {
> > + error_report_err(err);
> > + vm_stop(RUN_STATE_INTERNAL_ERROR);
> > + }
> > + ipl->iplb.ccw.netboot_start_addr = ipl->start_addr;
>
> Not sure whether it matters, but in case of early errors during
> load_netboot_image(), ipl->start_addr could be used uninitialized here.
> Maybe you should move the "ipl->start_addr = KERN_IMAGE_START;" there at
> the beginning of the function, to make it also the default value for the
> other error cases?
ipl->start_addr has already been set to some value in the realize
function (either the kernel entry address, or the bios address).
But that should not matter with the vm_stop() on error anyway, no?
