2023-10-02 10:03:37

by Paul Durrant

[permalink] [raw]
Subject: [PATCH v7 04/11] KVM: pfncache: base offset check on khva rather than gpa

From: Paul Durrant <[email protected]>

After a subsequent patch, the gpa may not always be set whereas khva will
(as long as the cache valid flag is also set).

No functional change intended.

Signed-off-by: Paul Durrant <[email protected]>
Reviewed-by: David Woodhouse <[email protected]>
---
Cc: Sean Christopherson <[email protected]>
Cc: David Woodhouse <[email protected]>
Cc: Paolo Bonzini <[email protected]>
---
virt/kvm/pfncache.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/virt/kvm/pfncache.c b/virt/kvm/pfncache.c
index 17afbb464a70..37bcb4399780 100644
--- a/virt/kvm/pfncache.c
+++ b/virt/kvm/pfncache.c
@@ -83,15 +83,18 @@ bool kvm_gpc_check(struct gfn_to_pfn_cache *gpc, unsigned long len)
if (!gpc->active)
return false;

- if ((gpc->gpa & ~PAGE_MASK) + len > PAGE_SIZE)
+ if (gpc->generation != slots->generation)
return false;

- if (gpc->generation != slots->generation || kvm_is_error_hva(gpc->uhva))
+ if (kvm_is_error_hva(gpc->uhva))
return false;

if (!gpc->valid)
return false;

+ if (offset_in_page(gpc->khva) + len > PAGE_SIZE)
+ return false;
+
return true;
}
EXPORT_SYMBOL_GPL(kvm_gpc_check);
--
2.39.2


2023-10-31 23:41:06

by Sean Christopherson

[permalink] [raw]
Subject: Re: [PATCH v7 04/11] KVM: pfncache: base offset check on khva rather than gpa

On Mon, Oct 02, 2023, Paul Durrant wrote:
> From: Paul Durrant <[email protected]>
>
> After a subsequent patch, the gpa may not always be set whereas khva will
> (as long as the cache valid flag is also set).

This holds true only because there are no users of KVM_GUEST_USES_PFN, and
because hva_to_pfn_retry() rather oddly adds the offset to a NULL khva.

I think it's time to admit using this to map PFNs into the guest is a bad idea
and rip out KVM_GUEST_USES_PFN before fully relying on khva.

https://lore.kernel.org/all/[email protected]

2023-11-02 18:12:44

by Paul Durrant

[permalink] [raw]
Subject: Re: [PATCH v7 04/11] KVM: pfncache: base offset check on khva rather than gpa

On 31/10/2023 23:40, Sean Christopherson wrote:
> On Mon, Oct 02, 2023, Paul Durrant wrote:
>> From: Paul Durrant <[email protected]>
>>
>> After a subsequent patch, the gpa may not always be set whereas khva will
>> (as long as the cache valid flag is also set).
>
> This holds true only because there are no users of KVM_GUEST_USES_PFN, and
> because hva_to_pfn_retry() rather oddly adds the offset to a NULL khva.
>
> I think it's time to admit using this to map PFNs into the guest is a bad idea
> and rip out KVM_GUEST_USES_PFN before fully relying on khva.
>
> https://lore.kernel.org/all/[email protected]

Is this something you want me to fix?

Paul

2023-11-03 23:11:10

by Sean Christopherson

[permalink] [raw]
Subject: Re: [PATCH v7 04/11] KVM: pfncache: base offset check on khva rather than gpa

On Thu, Nov 02, 2023, Paul Durrant wrote:
> On 31/10/2023 23:40, Sean Christopherson wrote:
> > On Mon, Oct 02, 2023, Paul Durrant wrote:
> > > From: Paul Durrant <[email protected]>
> > >
> > > After a subsequent patch, the gpa may not always be set whereas khva will
> > > (as long as the cache valid flag is also set).
> >
> > This holds true only because there are no users of KVM_GUEST_USES_PFN, and
> > because hva_to_pfn_retry() rather oddly adds the offset to a NULL khva.
> >
> > I think it's time to admit using this to map PFNs into the guest is a bad idea
> > and rip out KVM_GUEST_USES_PFN before fully relying on khva.
> >
> > https://lore.kernel.org/all/[email protected]
>
> Is this something you want me to fix?

Yes? I don't want to snowball your series, but I also really don't like the
confusion that is introduced by relying on khva while KVM_GUEST_USES_PFN is still
a thing.

Can you give it a shot, and then holler if it's a bigger mess than I'm anticipating?
I'm assuming/hoping it's a relatively small, one-off patch, but I haven't actually
dug through in-depth to figure out what all needs to change.