Re: [RFC PATCH] tests/qtest: properly initialise the vring used idx

["Alex Bennée" <alex.bennee@linaro.org>]

Peter Maydell <peter.maydell@linaro.org> writes:

> On Wed, 6 Apr 2022 at 18:36, Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>> Eric noticed while attempting to enable the vhost-user-blk-test for
>> Aarch64 that that things didn't work unless he put in a dummy
>> guest_malloc() at the start of the test. Without it
>> qvirtio_wait_used_elem() would assert when it reads a junk value for
>> idx resulting in:
>>
>>   qvirtqueue_get_buf: idx:2401 last_idx:0
>>   qvirtqueue_get_buf: 0x7ffcb6d3fe74, (nil)
>>   qvirtio_wait_used_elem: 3000000/0
>>   ERROR:../../tests/qtest/libqos/virtio.c:226:qvirtio_wait_used_elem: assertion failed (got_desc_idx == desc_idx): (50331648 == 0)
>>   Bail out! ERROR:../../tests/qtest/libqos/virtio.c:226:qvirtio_wait_used_elem: assertion failed (got_desc_idx == desc_idx): (50331648 == 0)
>>
>> What was actually happening is the guest_malloc() effectively pushed
>> the allocation of the vring into the next page which just happened to
>> have clear memory. After much tedious tracing of the code I could see
>> that qvring_init() does attempt initialise a bunch of the vring
>> structures but skips the vring->used.idx value. It is probably not
>> wise to assume guest memory is zeroed anyway. Once the ring is
>> properly initialised the hack is no longer needed to get things
>> working.
>
> Guest memory is generally zero at startup. Do we manage to
> hit the bit of memory at the start of the virt machine's RAM
> where we store the DTB ? (As you say, initializing the data
> structures is the right thing anyway.)

I don't know - where is the DTB loaded? Currently we are using the first
couple of pages in qtest because that where the qtest allocater is
initialised:

  static void *qos_create_machine_arm_virt(QTestState *qts)
  {
      QVirtMachine *machine = g_new0(QVirtMachine, 1);

      alloc_init(&machine->alloc, 0,
                 ARM_VIRT_RAM_ADDR,
                 ARM_VIRT_RAM_ADDR + ARM_VIRT_RAM_SIZE,
                 ARM_PAGE_SIZE);
      qvirtio_mmio_init_device(&machine->virtio_mmio, qts, VIRTIO_MMIO_BASE_ADDR,
                               VIRTIO_MMIO_SIZE);

      qos_create_generic_pcihost(&machine->bridge, qts, &machine->alloc);

      machine->obj.get_device = virt_get_device;
      machine->obj.get_driver = virt_get_driver;
      machine->obj.destructor = virt_destructor;
      return machine;
  }

I don't know if there is a more sane piece of memory we should be using.


>
> thanks
> -- PMM


-- 
Alex Bennée