| Message ID | 1422592273-4432-1-git-send-email-lersek@redhat.com |
|---|---|
| State | New |
| Headers | show |
Peter, On 01/30/15 05:31, Laszlo Ersek wrote: > Signed-off-by: Laszlo Ersek <lersek@redhat.com> > --- > hw/arm/virt.c | 32 ++++++++++++++++++++++++++++---- > 1 file changed, 28 insertions(+), 4 deletions(-) > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index 2353440..091e5ee 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -441,10 +441,27 @@ static void create_virtio_devices(const VirtBoardInfo *vbi, qemu_irq *pic) > int i; > hwaddr size = vbi->memmap[VIRT_MMIO].size; > > - /* Note that we have to create the transports in forwards order > - * so that command line devices are inserted lowest address first, > - * and then add dtb nodes in reverse order so that they appear in > - * the finished device tree lowest address first. > + /* 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 reserved; see eg. can you please s/reserved/reversed/? Result of over-editing, sorry. Thanks > + * 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. > */ > for (i = 0; i < NUM_VIRTIO_TRANSPORTS; i++) { > int irq = vbi->irqmap[VIRT_MMIO] + i; > @@ -453,6 +470,13 @@ static void create_virtio_devices(const VirtBoardInfo *vbi, qemu_irq *pic) > 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--) { > char *nodename; > int irq = vbi->irqmap[VIRT_MMIO] + i; >
On 30 January 2015 at 04:34, Laszlo Ersek <lersek@redhat.com> wrote: > Peter, > > On 01/30/15 05:31, Laszlo Ersek wrote: >> Signed-off-by: Laszlo Ersek <lersek@redhat.com> >> --- >> hw/arm/virt.c | 32 ++++++++++++++++++++++++++++---- >> 1 file changed, 28 insertions(+), 4 deletions(-) >> >> diff --git a/hw/arm/virt.c b/hw/arm/virt.c >> index 2353440..091e5ee 100644 >> --- a/hw/arm/virt.c >> +++ b/hw/arm/virt.c >> @@ -441,10 +441,27 @@ static void create_virtio_devices(const VirtBoardInfo *vbi, qemu_irq *pic) >> int i; >> hwaddr size = vbi->memmap[VIRT_MMIO].size; >> >> - /* Note that we have to create the transports in forwards order >> - * so that command line devices are inserted lowest address first, >> - * and then add dtb nodes in reverse order so that they appear in >> - * the finished device tree lowest address first. >> + /* 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 reserved; see eg. > > can you please s/reserved/reversed/? > > Result of over-editing, sorry. Sure, no problem. I also suggest I add this para: * * 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. -- PMM
On 02/03/15 12:51, Peter Maydell wrote: > On 30 January 2015 at 04:34, Laszlo Ersek <lersek@redhat.com> wrote: >> Peter, >> >> On 01/30/15 05:31, Laszlo Ersek wrote: >>> Signed-off-by: Laszlo Ersek <lersek@redhat.com> >>> --- >>> hw/arm/virt.c | 32 ++++++++++++++++++++++++++++---- >>> 1 file changed, 28 insertions(+), 4 deletions(-) >>> >>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c >>> index 2353440..091e5ee 100644 >>> --- a/hw/arm/virt.c >>> +++ b/hw/arm/virt.c >>> @@ -441,10 +441,27 @@ static void create_virtio_devices(const VirtBoardInfo *vbi, qemu_irq *pic) >>> int i; >>> hwaddr size = vbi->memmap[VIRT_MMIO].size; >>> >>> - /* Note that we have to create the transports in forwards order >>> - * so that command line devices are inserted lowest address first, >>> - * and then add dtb nodes in reverse order so that they appear in >>> - * the finished device tree lowest address first. >>> + /* 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 reserved; see eg. >> >> can you please s/reserved/reversed/? >> >> Result of over-editing, sorry. > > Sure, no problem. I also suggest I add this para: > * > * 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. Thanks. Do you want me to resubmit, or can you just squash it and observe that fact in a [peter.maydell@linaro.org: note about UUIDs] betwixt two other tags? Thanks Laszlo PS: yes, I love "betwixt". Whether I used it correctly above is secondary. ;)
On 3 February 2015 at 12:04, Laszlo Ersek <lersek@redhat.com> wrote: > On 02/03/15 12:51, Peter Maydell wrote: >> Sure, no problem. I also suggest I add this para: >> * >> * 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. > > Thanks. Do you want me to resubmit, or can you just squash it and > observe that fact in a [peter.maydell@linaro.org: note about UUIDs] > betwixt two other tags? No, I'll just fix it up as I queue the patch. -- PMM
diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 2353440..091e5ee 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -441,10 +441,27 @@ static void create_virtio_devices(const VirtBoardInfo *vbi, qemu_irq *pic) int i; hwaddr size = vbi->memmap[VIRT_MMIO].size; - /* Note that we have to create the transports in forwards order - * so that command line devices are inserted lowest address first, - * and then add dtb nodes in reverse order so that they appear in - * the finished device tree lowest address first. + /* 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 reserved; 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. */ for (i = 0; i < NUM_VIRTIO_TRANSPORTS; i++) { int irq = vbi->irqmap[VIRT_MMIO] + i; @@ -453,6 +470,13 @@ static void create_virtio_devices(const VirtBoardInfo *vbi, qemu_irq *pic) 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--) { char *nodename; int irq = vbi->irqmap[VIRT_MMIO] + i;
Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- hw/arm/virt.c | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-)