On 1/31/22 9:46 AM, Pierre Morel wrote:
On 1/14/22 21:38, Matthew Rosato wrote:
Use the associated vfio feature ioctl to enable interpretation for
devices
when requested. As part of this process, we must use the host function
handle rather than a QEMU-generated one -- this is provided as part of
the
ioctl payload.
I wonder if we should not explain here that having interpretation as a
default and silently fall back to interception allows backward
compatibility while allowing performence be chosing by default.
(You can say it better as I do :) )
Good suggestion, I'll think of something to add to the commit message.
@@ -1022,12 +1068,33 @@ static void s390_pcihost_plug(HotplugHandler
*hotplug_dev, DeviceState *dev,
set_pbdev_info(pbdev);
if (object_dynamic_cast(OBJECT(dev), "vfio-pci")) {
- pbdev->fh |= FH_SHM_VFIO;
+ /*
+ * By default, interpretation is always requested; if the
available
+ * facilities indicate it is not available, fallback to the
+ * intercept model.
s/intercept/interception/ ?
OK
+ */
+ if (pbdev->interp &&
!s390_has_feat(S390_FEAT_ZPCI_INTERP)) {
+ DPRINTF("zPCI interpretation facilities
missing.\n");
+ pbdev->interp = false;
+ }
+ if (pbdev->interp) {
+ rc = s390_pci_interp_plug(s, pbdev);
+ if (rc) {
+ error_setg(errp, "zpci interp plug failed: %d", rc);
+ return;
+ }
+ }
Can't we rearrange that as
if (pbdev->interp) {
if (s390_has_feat) {
} else {
}
}
Yep, sure
...
LGTM
With the corrections proposed by Thomas.
Mine... you see what you prefer.
Reviewed-by: Pierre Morel <pmorel@xxxxxxxxxxxxx>
Thanks!