Re: [Minios-devel] [UNIKRAFT PATCH RFC v2 3/7] plat/common: Introduce new platform bus

[Julien Grall <julien.grall@xxxxxxx>]

Hi,

On 25/10/2019 01:12, Justin He (Arm Technology China) wrote:
> > -----Original Message-----
> > From: Julien Grall <julien.grall@xxxxxxx>
> > Sent: Thursday, October 24, 2019 4:35 PM
> > To: Justin He (Arm Technology China) <Justin.He@xxxxxxx>; minios-
> > devel@xxxxxxxxxxxxxxxxxxxx; Santiago.Pagani@xxxxxxxxx
> > Cc: nd <nd@xxxxxxx>; Simon Kuenzer <simon.kuenzer@xxxxxxxxx>;
> > Sharan.Santhanam@xxxxxxxxx; Felipe Huici <felipe.huici@xxxxxxxxx>; Kaly
> > Xin (Arm Technology China) <Kaly.Xin@xxxxxxx>
> > Subject: Re: [UNIKRAFT PATCH RFC v2 3/7] plat/common: Introduce new
> > platform bus
> >
> > Hi,
> >
> > On 10/24/19 5:55 AM, Justin He (Arm Technology China) wrote:
> > > > > +
> > > > > +drv = pf_find_driver(&devid);
> > > > > +if (!drv) {
> > > > > +uk_pr_info("<no driver>\n");
> > > > > +return -ENODEV;
> > > > > +}
> > > > > +
> > > > > +uk_pr_info("driver %p\n", drv);
> > > > > +
> > > > > +/* qemu creates virtio devices in reverse order */
> > > > Why the order matters here? What's going to happen if you are using a
> > > > different backend that will chose a different order?
> > > Please see the create_virtio_devices() in qemu codes
> > >       for (i = 0; i < NUM_VIRTIO_TRANSPORTS; i++) {
> > >           int irq = vms->irqmap[VIRT_MMIO] + i;
> > >           hwaddr base = vms->memmap[VIRT_MMIO].base + i * size;
> > >
> > >           sysbus_create_simple("virtio-mmio", base, pic[irq]);
> > >       }
> > >
> > >       /* We add dtb nodes in reverse order so that they appear in the
> > finished
> > >        * device tree lowest address first.
> > >        *
> > >        * Note that this mapping is independent of the loop above. The
> > previous
> > >        * loop influences virtio device to virtio transport assignment, whereas
> > >        * this loop controls how virtio transports are laid out in the dtb.
> > >        */
> > >       for (i = NUM_VIRTIO_TRANSPORTS - 1; i >= 0; i--) {
> > >
> > > So qemu ^ will create max (32) virtio mmio devices and in reverse order.
> > This tells me what QEMU does *not* why Unikraft needs to do it.
> >
> > AFAICT, both Linux and FreeBSD does need to search for virtio,mmio node
> > in reverse order. So why does Unikraft need it? In other word, what will
> > happen if you do the "wrong" way?
> Sorry, I don't quite understand your concerns here.
> If Unikraft didn’t search it in reverse order, how can it probe the correct
> Virtio-mmio device? Could you please elaborate it more, thanks
QEMU actually gives a good example why relying on ordering may lead to unwanted 
behavior:
    /* We create the transports in forwards order. Since qbus_realize()
     * prepends (not appends) new child buses, the incrementing loop below will
     * create a list of virtio-mmio buses with decreasing base addresses.
     *
     * When a -device option is processed from the command line,
     * qbus_find_recursive() picks the next free virtio-mmio bus in forwards
     * order. The upshot is that -device options in increasing command line
     * order are mapped to virtio-mmio buses with decreasing base addresses.
     *
     * When this code was originally written, that arrangement ensured that the
     * guest Linux kernel would give the lowest "name" (/dev/vda, eth0, etc) to
     * the first -device on the command line. (The end-to-end order is a
     * function of this loop, qbus_realize(), qbus_find_recursive(), and the
     * guest kernel's name-to-address assignment strategy.)
     *
     * Meanwhile, the kernel's traversal seems to have been reversed; see eg.
     * the message, if not necessarily the code, of commit 70161ff336.
     * Therefore the loop now establishes the inverse of the original intent.
     *
     * Unfortunately, we can't counteract the kernel change by reversing the
     * loop; it would break existing command lines.
     *
     * In any case, the kernel makes no guarantee about the stability of
     * enumeration order of virtio devices (as demonstrated by it changing
     * between kernel versions). For reliable and stable identification
     * of disks users must use UUIDs or similar mechanisms.
     */

More generally, Device-Tree does not guarantee that devices will be laid out in 
the correct order. For instance, if you are using a non-QEMU backend you may 
have a different order in the device-tree.
As this is common code, you definitely can't rely on ordering. But it would also 
clearly be wrong if you implement the probe ordering based on your backend.
In your solution, can you explain how you expect Unikraft to find a virtio 
device?

Cheers,

--
Julien Grall

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel