Re: [PATCH 23/32] KVM: s390: pci: handle refresh of PCI translations

Pierre Morel
2021-12-14T16:58:37Z

On 12/7/21 21:57, Matthew Rosato wrote:
> Add a routine that will perform a shadow operation between a guest
> and host IOAT. A subsequent patch will invoke this in response to
> an 04 RPCIT instruction intercept.
>
> Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com>
> ---
> arch/s390/include/asm/kvm_pci.h | 1 +
> arch/s390/include/asm/pci_dma.h | 1 +
> arch/s390/kvm/pci.c | 191 ++++++++++++++++++++++++++++++++
> arch/s390/kvm/pci.h | 4 +-
> 4 files changed, 196 insertions(+), 1 deletion(-)
>
> diff --git a/arch/s390/include/asm/kvm_pci.h b/arch/s390/include/asm/kvm_pci.h
> index 254275399f21..97e3a369135d 100644
> --- a/arch/s390/include/asm/kvm_pci.h
> +++ b/arch/s390/include/asm/kvm_pci.h
> @@ -30,6 +30,7 @@ struct kvm_zdev_ioat {
> struct kvm_zdev {
> struct zpci_dev *zdev;
> struct kvm *kvm;
> + u64 rpcit_count;
> struct kvm_zdev_ioat ioat;
> struct zpci_fib fib;
> };
> diff --git a/arch/s390/include/asm/pci_dma.h b/arch/s390/include/asm/pci_dma.h
> index e1d3c1d3fc8a..0ca15e5db3d9 100644
> --- a/arch/s390/include/asm/pci_dma.h
> +++ b/arch/s390/include/asm/pci_dma.h
> @@ -52,6 +52,7 @@ enum zpci_ioat_dtype {
> #define ZPCI_TABLE_ENTRIES (ZPCI_TABLE_SIZE / ZPCI_TABLE_ENTRY_SIZE)
> #define ZPCI_TABLE_PAGES (ZPCI_TABLE_SIZE >> PAGE_SHIFT)
> #define ZPCI_TABLE_ENTRIES_PAGES (ZPCI_TABLE_ENTRIES * ZPCI_TABLE_PAGES)
> +#define ZPCI_TABLE_ENTRIES_PER_PAGE (ZPCI_TABLE_ENTRIES / ZPCI_TABLE_PAGES)
>
> #define ZPCI_TABLE_BITS 11
> #define ZPCI_PT_BITS 8
> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
> index a1c0c0881332..858c5ecdc8b9 100644
> --- a/arch/s390/kvm/pci.c
> +++ b/arch/s390/kvm/pci.c
> @@ -123,6 +123,195 @@ int kvm_s390_pci_aen_init(u8 nisc)
> return rc;
> }

...snip...

> +
> +int kvm_s390_pci_refresh_trans(struct kvm_vcpu *vcpu, unsigned long req,
> + unsigned long start, unsigned long size)
> +{
> + struct zpci_dev *zdev;
> + u32 fh;
> + int rc;
> +
> + /* If the device has a SHM bit on, let userspace take care of this */
> + fh = req >> 32;
> + if ((fh & aift.mdd) != 0)
> + return -EOPNOTSUPP;

I think you should make this check in the caller.

> +
> + /* Make sure this is a valid device associated with this guest */
> + zdev = get_zdev_by_fh(fh);
> + if (!zdev || !zdev->kzdev || zdev->kzdev->kvm != vcpu->kvm)
> + return -EINVAL;
> +
> + /* Only proceed if the device is using the assist */
> + if (zdev->kzdev->ioat.head[0] == 0)
> + return -EOPNOTSUPP;

Using the assist means using interpretation over using interception and
legacy vfio-pci. right?

> +
> + rc = dma_table_shadow(vcpu, zdev, start, size);
> + if (rc > 0)
> + rc = zpci_refresh_trans((u64) zdev->fh << 32, start, size);

Here you lose the status reported by the hardware.
You should directly use __rpcit(fn, addr, range, &status);

> + zdev->kzdev->rpcit_count++;
> +
> + return rc;
> +}
> +
> /* Modify PCI: Register floating adapter interruption forwarding */
> static int kvm_zpci_set_airq(struct zpci_dev *zdev)
> {
> @@ -590,4 +779,6 @@ void kvm_s390_pci_init(void)
> {
> spin_lock_init(&aift.gait_lock);
> mutex_init(&aift.lock);
> +
> + WARN_ON(zpci_get_mdd(&aift.mdd));
> }
> diff --git a/arch/s390/kvm/pci.h b/arch/s390/kvm/pci.h
> index 3c86888fe1b3..d252a631b693 100644
> --- a/arch/s390/kvm/pci.h
> +++ b/arch/s390/kvm/pci.h
> @@ -33,6 +33,7 @@ struct zpci_aift {
> struct kvm_zdev **kzdev;
> spinlock_t gait_lock; /* Protects the gait, used during AEN forward */
> struct mutex lock; /* Protects the other structures in aift */
> + u32 mdd;
> };
>
> static inline struct kvm *kvm_s390_pci_si_to_kvm(struct zpci_aift *aift,
> @@ -47,7 +48,8 @@ struct zpci_aift *kvm_s390_pci_get_aift(void);
>
> int kvm_s390_pci_aen_init(u8 nisc);
> void kvm_s390_pci_aen_exit(void);
> -
> +int kvm_s390_pci_refresh_trans(struct kvm_vcpu *vcpu, unsigned long req,
> + unsigned long start, unsigned long end);
> void kvm_s390_pci_init(void);
>
> #endif /* __KVM_S390_PCI_H */
>

--
Pierre Morel
IBM Lab Boeblingen