RE: [PATCH 1/3] shared-mem: introduce page alignment restrictions

Parav Pandit
2025-03-04T10:58:17Z

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Tuesday, March 4, 2025 4:23 PM
>
> On Sun, Feb 23, 2025 at 05:21:00AM +0000, Parav Pandit wrote:
> > Overall extending pci cap structure is not good idea even though it may
> appear as small change for below reasons.
> >
> > 1. PCI spec already ran out of total bytes that can be stored in the capability
> section. Extending it will not help.
> > A virtio level extended capability is not good either due to below guidance
> from PCI-SIG.
> >
> > 2. PCI spec highly discouraged putting vendor specific bits like this in the
> capability section.
> > Citation: "It is strongly recommended that PCI Express devices place
> > no registers in Configuration Space other than those in headers or Capability
> structures architected by applicable PCI specifications."
>
>
> We can argue about new capabilities, but this is not doing it - it is merely
> extending the existing one.
>
This patch is not even extending it. it is reusing the pad field, which is good.
Extending capability is problematic.

> So I do not really get any of these arguments.
>
I asked him to explore what all options did he consider.

>
> I'd be open to an alternative way to discover capabilities, as long as we don't
> do that, I don't really see a reason to block minor tweaks like this one.
>
I am not blocking it. would like to know what other options were considered if the shared memory is going to grow or its just one-off field that can live in PCI capability field.
I suggest, that this should be done without negotiating a feature bit. Because the device is just telling the restrictions...

> At the same time, I am not sure why this is a capability at all.
> why not in config space?
>
When you say "config space", you meant "virtio config space" and not "pci config space".
Assuming yes, I believe the problem is, Sergio needs to add this field in ALL 19+ devices config space, which is not good.

> --
> MST