On Fri, Aug 8, 2025 at 12:55 PM Akihiko Odaki
<od...@rsg.ci.i.u-tokyo.ac.jp> wrote:
>
> On 2025/08/08 5:18, Paolo Abeni wrote:
> > On 7/26/25 1:52 PM, Akihiko Odaki wrote:
> >> On 2025/07/24 4:31, Paolo Abeni wrote:
> >>> @@ -1477,6 +1509,13 @@ int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy,
> >>> return virtio_pci_add_mem_cap(proxy, &cap.cap);
> >>> }
> >>>
> >>> +static int virtio_pci_select_max(const VirtIODevice *vdev)
> >>> +{
> >>> + return virtio_features_use_ex(vdev->host_features_ex) ?
> >>> + VIRTIO_FEATURES_NU32S :
> >>> + 2;
> >>
> >> This function could be simplified by replacing VIRTIO_FEATURES_NU32S
> >> without any functional difference:
Did you mean using VIRTIO_FEATURES_NU32S instead?
> >>
> >> 1. For writes: virtio_set_features_ex() already ignores extended
> >> features when !virtio_features_use_ex(vdev->host_features_ex)
> >> 2. For reads: When !virtio_features_use_ex(vdev->host_features_ex), the
> >> upper bits of host_features_ex are zero, and guest_features upper bits
> >> remain zero (since they can't be set per point 1)
I think it depends on the compatibility work which hasn't been done in
this series.
> >>
> >> So the conditional logic is redundant here.
See below
> >
> > This is to satisfy a request from Jason:
> >
> > https://lists.gnu.org/archive/html/qemu-devel/2025-07/msg05291.html
> > https://lists.gnu.org/archive/html/qemu-devel/2025-07/msg05423.html
> >
> > I agree there will not be functional differences always accessing the
> > full space, but the guest could still be able to notice, i.e. the
> > extended space will be zeroed on read with that patched qemu and
> > untouched by the current code and this patch. To be on the safe side I
> > think it would be better to avoid such difference, as suggested by Jason.
> >
> > Does the above make sense to you?
>
> By functional, I meant the functionality of QEMU, visible to the guest,
> rather than the whole system including the guest, visible to the end
> user. The guest cannot notice the difference because the extended space
> is zero on read even without the conditional, which is described as
> point 2 in the previous email.
I'm not sure I understand this correctly. But it doesn't harm here consider:
1) it's the entry point from the guest, checking and failing early is
better than depending on the low layer functions
2) we have checks in several layers (both virtio-pci and virtio core).
And it looks like a must for at least GF:
case VIRTIO_PCI_COMMON_GF:
if (proxy->gfselect < virtio_pci_select_max(vdev)) {
uint64_t features[VIRTIO_FEATURES_NU64S];
int i;
proxy->guest_features[proxy->gfselect] = val;
Thanks
>
> > 2. For reads: When !virtio_features_use_ex(vdev->host_features_ex),
> > the upper bits of host_features_ex are zero, and guest_features upper
> > bits remain zero (since they can't be set per point 1)
>
> Regards,
> Akihiko Odaki
>
