Messages in this thread | | | Date | Tue, 24 Apr 2018 08:29:19 +0200 | From | Gerd Hoffmann <> | Subject | Re: [PATCH 1/3] sample: vfio mdev display - host device |
| |
Hi,
> > +/* pci ids */ > > +#define MDPY_PCI_VENDOR_ID 0x1b36 /* redhat */ > > +#define MDPY_PCI_DEVICE_ID 0x00f0 > > I don't see this on pci-ids, so I assume we're just squatting on an > ID. How do we do that without risking that we don't interfere with > some future user? Are we relying on this being a non-default sample > device? Should we just ask for an allocation?
It's grabbed from qemu id range. Allocating one is probably a good idea even for a sample device.
> > +#define MDPY_PCI_SUBVENDOR_ID PCI_SUBVENDOR_ID_REDHAT_QUMRANET > > +#define MDPY_PCI_SUBDEVICE_ID PCI_SUBDEVICE_ID_QEMU > > + > > +/* pci cfg space offsets for fb config (dword) */ > > +#define MDPY_FORMAT_OFFSET 0x40 > > +#define MDPY_WIDTH_OFFSET 0x44 > > +#define MDPY_HEIGHT_OFFSET 0x48 > > As I understand, these are just registers in PCI config space outside > of any capabilities. Wouldn't it be more correct to put these within a > vendor defined capability?
Can do that.
> > + region_info->size = mdev_state->memsize; > > + region_info->flags = (VFIO_REGION_INFO_FLAG_READ | > > + VFIO_REGION_INFO_FLAG_WRITE | > > + VFIO_REGION_INFO_FLAG_MMAP); > > This doesn't appear to be true, the read and write functions call the > access function which only handles the config space region. Are these > really mmap-only regions?
Yes, they are mmap-only.
> read/write access support is often useful > for tracing and debugging, QEMU will break if x-no-mmap=on is used.
Hmm, can look into adding that, should not be that difficuilt after all.
cheers, Gerd
| |