Re: [SeaBIOS] [Qemu-devel] [PATCH] boot: fix path pattern of scsi device
On Tue, May 28, 2013 at 10:06:51AM +0200, Paolo Bonzini wrote:
> Il 28/05/2013 09:40, Amos Kong ha scritto:
> > bootindex parameter of scsi device doesn't work, it causes
> > by wrong pattern in seabios.
> >
> > qemu passes the following firmware dev_path to seabios:
> > /pci@i0cf8/scsi@4/virtio-scsi-device/channel@0/disk@0,0
>
> No, this is another unexpected change due to the virtio refactoring in
> QEMU. The right fix is in QEMU, by adding a get_fw_dev_path
> implementation in hw/virtio/virtio-bus.c.
Hi Paolo,
We could not fix this by implementing get_fw_dev_path in
hw/virtio/virtio-bus.c
virtio-bus is the parent bus of scsi-bus, scsibus_get_fw_dev_path()
will be called after called virtio_bus_get_fw_dev_path().
str0 = returns of parents of virtio-bus /* eg: /pci@i0cf8/scsi@4 */
str1 = return of virtio_bus_get_fw_dev_path() /* eg: .. */
str2 = scsibus_get_fw_dev_path() /* eg: channel@0/disk@0,0 */
The final fw_dev_path should be $str0/$str1/$str2
| static int qdev_get_fw_dev_path_helper(DeviceState *dev, char *p, int size)
| {
| int l = 0;
|
| if (dev && dev->parent_bus) {
| char *d;
| l = qdev_get_fw_dev_path_helper(dev->parent_bus->parent, p, size);
| d = bus_get_fw_dev_path(dev->parent_bus, dev);
| if (d) {
| l += snprintf(p + l, size - l, "%s", d);
| g_free(d);
if we implement virtio_bus_get_fw_dev_path(), the return value will be set to
$str1
| } else {
| l += snprintf(p + l, size - l, "%s",
object_get_typename(OBJECT(dev)));
Currently we didn't implement virtio_bus_get_fw_dev_path(), so
'virtio-scsi-device' was set to $str1
| }
|
However, we need to change the pattern.
Amos.
> We fixed it already for migration paths, it should be easy to do the
> same for this.
>
> Please Cc qemu-sta...@nongnu.org when sending the QEMU patch.
>
> Thanks,
>
> Paolo
>
> > Signed-off-by: Amos Kong
> > ---
> > src/boot.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/boot.c b/src/boot.c
> > index c308602..cd9d784 100644
> > --- a/src/boot.c
> > +++ b/src/boot.c
> > @@ -138,10 +138,11 @@ int bootprio_find_scsi_device(struct pci_device *pci,
> > int target, int lun)
> > if (!pci)
> > // support only pci machine for now
> > return -1;
> > -// Find scsi drive - for example: /pci@i0cf8/scsi@5/channel@0/disk@1,0
> > +/* Find scsi drive - for example:
> > + /pci@i0cf8/scsi@5/virtio-scsi-device/channel@0/disk@1,0 */
> > char desc[256], *p;
> > p = build_pci_path(desc, sizeof(desc), "*", pci);
> > -snprintf(p, desc+sizeof(desc)-p, "/*@0/*@%d,%d", target, lun);
> > +snprintf(p, desc+sizeof(desc)-p, "/*/*@0/*@%d,%d", target, lun);
> > return find_prio(desc);
> > }
> >
> >
___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [Qemu-devel] [PATCH] boot: fix path pattern of scsi device
Il 28/05/2013 11:35, Amos Kong ha scritto:
> On Tue, May 28, 2013 at 10:06:51AM +0200, Paolo Bonzini wrote:
>> Il 28/05/2013 09:40, Amos Kong ha scritto:
>>> bootindex parameter of scsi device doesn't work, it causes
>>> by wrong pattern in seabios.
>>>
>>> qemu passes the following firmware dev_path to seabios:
>>> /pci@i0cf8/scsi@4/virtio-scsi-device/channel@0/disk@0,0
>>
>> No, this is another unexpected change due to the virtio refactoring in
>> QEMU. The right fix is in QEMU, by adding a get_fw_dev_path
>> implementation in hw/virtio/virtio-bus.c.
>
> Hi Paolo,
>
> We could not fix this by implementing get_fw_dev_path in
> hw/virtio/virtio-bus.c
>
> virtio-bus is the parent bus of scsi-bus, scsibus_get_fw_dev_path()
> will be called after called virtio_bus_get_fw_dev_path().
>
> str0 = returns of parents of virtio-bus /* eg: /pci@i0cf8/scsi@4 */
> str1 = return of virtio_bus_get_fw_dev_path() /* eg: .. */
> str2 = scsibus_get_fw_dev_path() /* eg: channel@0/disk@0,0 */
>
> The final fw_dev_path should be $str0/$str1/$str2
>
> | static int qdev_get_fw_dev_path_helper(DeviceState *dev, char *p, int size)
> | {
> | int l = 0;
> |
> | if (dev && dev->parent_bus) {
> | char *d;
> | l = qdev_get_fw_dev_path_helper(dev->parent_bus->parent, p, size);
> | d = bus_get_fw_dev_path(dev->parent_bus, dev);
> | if (d) {
> | l += snprintf(p + l, size - l, "%s", d);
> | g_free(d);
>
> if we implement virtio_bus_get_fw_dev_path(), the return value will be set to
> $str1
>
> | } else {
> | l += snprintf(p + l, size - l, "%s",
> object_get_typename(OBJECT(dev)));
>
> Currently we didn't implement virtio_bus_get_fw_dev_path(), so
> 'virtio-scsi-device' was set to $str1
>
> | }
> |
>
> However, we need to change the pattern.
Turn this:
l += snprintf(p + l, size - l, "%s",
object_get_typename(OBJECT(dev)));
into the default implementation of get_fw_dev_path (in TYPE_BUS),
and change qdev_get_fw_dev_path_helper to
if (d) {
l += snprintf(p + l, size - l, "%s", d);
g_free(d);
} else {
return l;
}
Then virtio_bus_get_fw_dev_path can just return NULL.
Paolo
> Amos.
>
>> We fixed it already for migration paths, it should be easy to do the
>> same for this.
>>
>> Please Cc qemu-sta...@nongnu.org when sending the QEMU patch.
>>
>> Thanks,
>>
>> Paolo
>>
>>> Signed-off-by: Amos Kong
>>> ---
>>> src/boot.c | 5 +++--
>>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/boot.c b/src/boot.c
>>> index c308602..cd9d784 100644
>>> --- a/src/boot.c
>>> +++ b/src/boot.c
>>> @@ -138,10 +138,11 @@ int bootprio_find_scsi_device(struct pci_device *pci,
>>> int target, int lun)
>>> if (!pci)
>>> // support only pci machine for now
>>> return -1;
>>> -// Find scsi drive - for example: /pci@i0cf8/scsi@5/channel@0/disk@1,0
>>> +/* Find scsi drive - for example:
>>> + /pci@i0cf8/scsi@5/virtio-scsi-device/channel@0/disk@1,0 */
>>> char desc[256], *p;
>>> p = build_pci_path(desc, sizeof(desc), "*", pci);
>>> -snprintf(p, desc+sizeof(desc)-p, "/*@0/*@%d,%d", target, lun);
>>> +snprintf(p, desc+sizeof(desc)-p, "/*/*@0/*@%d,%d", target, lun);
>>> return find_prio(desc);
>>> }
>>>
>>>
___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [Qemu-devel] [PATCH] boot: fix path pattern of scsi device
On 05/28/13 10:06, Paolo Bonzini wrote: > Il 28/05/2013 09:40, Amos Kong ha scritto: >> bootindex parameter of scsi device doesn't work, it causes >> by wrong pattern in seabios. >> >> qemu passes the following firmware dev_path to seabios: >> /pci@i0cf8/scsi@4/virtio-scsi-device/channel@0/disk@0,0 > > No, this is another unexpected change due to the virtio refactoring in > QEMU. The right fix is in QEMU, by adding a get_fw_dev_path > implementation in hw/virtio/virtio-bus.c. > > We fixed it already for migration paths, it should be easy to do the > same for this. > > Please Cc qemu-sta...@nongnu.org when sending the QEMU patch. > > Thanks, > > Paolo Ahhh. I was super confused by this patch initially. Amos, when posting a patch to both lists, please add the project name to the bracketed bag-of-tags in the subject, like [SeaBIOS PATCH] boot: fix path pattern of scsi device I saw this message first on qemu-devel, and until I noticed "src/boot.c" I was kind of confused whom you want to adapt to whom, and in what direction Paolo argues against it. So, virtio refactoring in QEMU (care to name a commit or release?) changed the OpenFirmware device path exported for virtio-scsi devices under the boot order fw_cfg key. This patch intended to adapt SeaBIOS to recognize the new OFW devpath. Under this approach I would have to update QemuBootOrder.c in OVMF in parallel, so that it accepts both old and new style OFW devpaths for virtio-scsi. However Paolo says the new style OFW devpath should be fixed (eliminated) in qemu, and consumers shouldn't notice any change in the long term. And I won't have to change QemuBootOrder.c. Right? Thanks! Laszlo ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [Qemu-devel] [PATCH] boot: fix path pattern of scsi device
On Tue, May 28, 2013 at 12:26:34PM +0200, Laszlo Ersek wrote: > On 05/28/13 10:06, Paolo Bonzini wrote: > > Il 28/05/2013 09:40, Amos Kong ha scritto: > >> bootindex parameter of scsi device doesn't work, it causes > >> by wrong pattern in seabios. > >> > >> qemu passes the following firmware dev_path to seabios: > >> /pci@i0cf8/scsi@4/virtio-scsi-device/channel@0/disk@0,0 > > > > No, this is another unexpected change due to the virtio refactoring in > > QEMU. The right fix is in QEMU, by adding a get_fw_dev_path > > implementation in hw/virtio/virtio-bus.c. > > > > We fixed it already for migration paths, it should be easy to do the > > same for this. > > > > Please Cc qemu-sta...@nongnu.org when sending the QEMU patch. > > > > Thanks, > > > > Paolo > > Ahhh. I was super confused by this patch initially. > > Amos, when posting a patch to both lists, please add the project name to > the bracketed bag-of-tags in the subject, like > > [SeaBIOS PATCH] boot: fix path pattern of scsi device Sorry for the mistiness. > I saw this message first on qemu-devel, and until I noticed "src/boot.c" > I was kind of confused whom you want to adapt to whom, and in what > direction Paolo argues against it. > > So, virtio refactoring in QEMU (care to name a commit or release?) > changed the OpenFirmware device path exported for virtio-scsi devices > under the boot order fw_cfg key. This patch intended to adapt SeaBIOS to > recognize the new OFW devpath. Under this approach I would have to > update QemuBootOrder.c in OVMF in parallel, so that it accepts both old > and new style OFW devpaths for virtio-scsi. > > However Paolo says the new style OFW devpath should be fixed > (eliminated) in qemu, and consumers shouldn't notice any change in the > long term. And I won't have to change QemuBootOrder.c. Right? Just sent a qemu patch to fix this problem. We will keep original style devpath. > Thanks! > Laszlo -- Amos. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
