Message-ID: <87bl9sunnw.wl-maz@kernel.org>
Date:   Mon, 03 May 2021 11:17:23 +0100
From:   Marc Zyngier <maz@...nel.org>
To:     Vikram Sethi <vsethi@...dia.com>
Cc:     Shanker Donthineni <sdonthineni@...dia.com>,
        Alex Williamson <alex.williamson@...hat.com>,
        Will Deacon <will@...nel.org>,
        Catalin Marinas <catalin.marinas@....com>,
        Christoffer Dall <christoffer.dall@....com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "kvmarm@...ts.cs.columbia.edu" <kvmarm@...ts.cs.columbia.edu>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        Jason Sequeira <jsequeira@...dia.com>
Subject: Re: [RFC 1/2] vfio/pci: keep the prefetchable attribute of a BAR region in VMA

Hi Vikram,

On Sun, 02 May 2021 18:56:31 +0100,
Vikram Sethi <vsethi@...dia.com> wrote:
> 
> Hi Marc, 
> 
> > From: Marc Zyngier <maz@...nel.org>
> > Hi Vikram,
> > 
>  
> > The problem I see is that we have VM and userspace being written in terms
> > of Write-Combine, which is:
> > 
> > - loosely defined even on x86
> > 
> > - subject to interpretations in the way it maps to PCI
> > 
> > - has no direct equivalent in the ARMv8 collection of memory
> >   attributes (and Normal_NC comes with speculation capabilities which
> >   strikes me as extremely undesirable on arbitrary devices)
> 
> If speculation with Normal NC to prefetchable BARs in devices was a
> problem, those devices would already be broken in baremetal with
> ioremap_wc on arm64, and we would need quirks there to not do Normal
> NC for them but Device GRE, and if such a quirk was needed on
> baremetal, it could be picked up by vfio/KVM as well. But we haven't
> seen any broken devices doing wc on baremetal on ARM64, have we?

The lack of evidence does not equate to a proof, and your devices not
misbehaving doesn't mean it is the right thing, specially when we have
such a wide range of CPU and interconnect implementation. Which is why
I really want an answer at the architecture level. Not a "it works for
me" type of answer.

Furthermore, as I replied to Shanker in a separate email, what
Linux/arm64 does is pretty much irrelevant. KVM/arm64 implements the
ARMv8 architecture, and it is at that level that we need to solve the
problem.

If, by enumerating the properties of Prefetchable, you can show that
they are a strict superset of Normal_NC, I'm on board. I haven't seen
such an enumeration so far.

> I know we have tested NICs write combining on arm64 in baremetal, as
> well as GPU and NVMe CMB without issues.
> 
> Further, I don't see why speculation to non cacheble would be an
> issue if prefetch without side effects is allowed by the device,
> which is what a prefetchable BAR is.
> If it is an issue for a device I would consider that a bug already needing a quirk in
> Baremetal/host kernel already. 
> From PCI spec " A prefetchable address range may have write side effects, 
> but it may not have read side effects."

Right, so we have made a small step in the direction of mapping
"prefetchable" onto "Normal_NC", thanks for that. What about all the
other properties (unaligned accesses, ordering, gathering)?

> > How do we translate this into something consistent? I'd like to see an actual
> > description of what we *really* expect from WC on prefetchable PCI regions,
> > turn that into a documented definition agreed across architectures, and then
> > we can look at implementing it with one memory type or another on arm64.
> > 
> > Because once we expose that memory type at S2 for KVM guests, it
> > becomes ABI and there is no turning back. So I want to get it right once and
> > for all.
> > 
> I agree that we need a precise definition for the Linux ioremap_wc
> API wrt what drivers (kernel and userspace) can expect and whether
> memset/memcpy is expected to work or not and whether aligned
> accesses are a requirement.
> To the extent ABI is set, I would think that the ABI is also already
> set in the host kernel for arm64 WC = Normal NC, so why should that
> not also be the ABI for same driver in VMs.

KVM is an implementation of the ARM architecture, and doesn't really
care about what WC is. If we come to the conclusion that Normal_NC is
the natural match for Prefetchable attributes, than we're good and we
can have Normal_NC being set by userspace, or even VFIO. But I don't
want to set it only because "it works when bare-metal Linux uses it".
Remember KVM doesn't only run Linux as guests.

	M.

-- 
Without deviation from the norm, progress is not possible.

Powered by blists - more mailing lists