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
