Hi Andre,
On Wed, Oct 06, 2021 at 04:11:28PM +0100, Andre Przywara wrote:
> On Mon, 13 Sep 2021 16:44:12 +0100
> Alexandru Elisei <alexandru.elisei@xxxxxxx> wrote:
>
> > Now that we keep track of the real size of MSIX table and PBA, print an
> > error when the guest tries to write to an offset which is not inside the
> > correct regions.
>
> What is the actual purpose of this message? Is there anything the user can
> do about it? Shouldn't we either abort the guest or inject an error
> condition back into KVM or the guest instead?
The only way for the error message to trigger is in case of a programming error
in kvmtool - when access to the PBA BAR is enabled by the guest, kvmtool will
register the vfio_pci_msix_pba_access callback for the memory region
[pba->guest_phys_addr, pba->guest_phys_addr + pba->size) in
vfio_pci_bar_activate().
I believe kvmtool should definitely print something when it detects a
programming error. The question now becomes if kvmtool should die or continue
running the VM, as the VM can run just fine even if emulation for assigned
devices is malfunctioning. I would rather give the user the chance to safely
shutdown the VM instead of killing it outright.
Thanks,
Alex
>
> Cheers,
> Andre
>
> >
> > Signed-off-by: Alexandru Elisei <alexandru.elisei@xxxxxxx>
> > ---
> > vfio/pci.c | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/vfio/pci.c b/vfio/pci.c
> > index 7781868..a6d0408 100644
> > --- a/vfio/pci.c
> > +++ b/vfio/pci.c
> > @@ -249,6 +249,11 @@ static void vfio_pci_msix_pba_access(struct kvm_cpu
> > *vcpu, u64 addr, u8 *data, u64 offset = addr - pba->guest_phys_addr;
> > struct vfio_device *vdev = container_of(pdev, struct
> > vfio_device, pci);
> > + if (offset >= pba->size) {
> > + vfio_dev_err(vdev, "access outside of the MSIX PBA");
> > + return;
> > + }
> > +
> > if (is_write)
> > return;
> >
> > @@ -269,6 +274,10 @@ static void vfio_pci_msix_table_access(struct
> > kvm_cpu *vcpu, u64 addr, u8 *data, struct vfio_device *vdev =
> > container_of(pdev, struct vfio_device, pci);
> > u64 offset = addr - pdev->msix_table.guest_phys_addr;
> > + if (offset >= pdev->msix_table.size) {
> > + vfio_dev_err(vdev, "access outside of the MSI-X table");
> > + return;
> > + }
> >
> > size_t vector = offset / PCI_MSIX_ENTRY_SIZE;
> > off_t field = offset % PCI_MSIX_ENTRY_SIZE;
>
