diff mbox series

[3/6] KVM: Documentation: Add the missing ptep in kvm_mmu_page

Message ID 20230618000856.1714902-4-mizhang@google.com (mailing list archive)
State New, archived
Headers show
Series KVM: Documentation: Update document description for kvm_mmu_page and kvm_mmu_page_role | expand

Commit Message

Mingwei Zhang June 18, 2023, 12:08 a.m. UTC
Add the missing ptep in kvm_mmu_page description. ptep is used when TDP MMU
is enabled and it shares the storage with parent_ptes. Update the doc to
help readers to get up-to-date info.

Signed-off-by: Mingwei Zhang <mizhang@google.com>
---
 Documentation/virt/kvm/x86/mmu.rst | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Kai Huang June 22, 2023, 8:32 a.m. UTC | #1
On Sun, 2023-06-18 at 00:08 +0000, Mingwei Zhang wrote:
> Add the missing ptep in kvm_mmu_page description. ptep is used when TDP MMU
> is enabled and it shares the storage with parent_ptes. Update the doc to
> help readers to get up-to-date info.
> 
> Signed-off-by: Mingwei Zhang <mizhang@google.com>
> ---
>  Documentation/virt/kvm/x86/mmu.rst | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/virt/kvm/x86/mmu.rst b/Documentation/virt/kvm/x86/mmu.rst
> index 149dd3cba48f..36bfe0fe02bb 100644
> --- a/Documentation/virt/kvm/x86/mmu.rst
> +++ b/Documentation/virt/kvm/x86/mmu.rst
> @@ -236,6 +236,10 @@ Shadow pages contain the following information:
>      parent_ptes points at this single spte, otherwise, there exists multiple
>      sptes pointing at this page and (parent_ptes & ~0x1) points at a data
>      structure with a list of parent sptes.
> +  ptep:
> +    Pointer to the parent spte when TDP MMU is enabled. 
> 

IMHO "parent spte" alone _may_ be confusing.  I think it's better to explicitly
mention "pointing to this page" similar to the "parent_ptes" above.

Also, I think "when TDP MMU is enabled" isn't strictly true, depending on what
does "when TDP MMU is enabled mean".  E.g., when tdp_mmu_enabled module
parameter is true, we can still have a nested EPT shadow page from L2 which
won't use this either IIUC.

> In TDP MMU, each
> +    shadow page will have at most one parent. Note that this field is a
> +    union with parent_ptes.

Also, perhaps "have at most one parent" can be more precise: only root page has
no parent, while other non-root pages always have one parent SPTE pointing to
each of them.
Mingwei Zhang June 26, 2023, 5:29 p.m. UTC | #2
On Thu, Jun 22, 2023, Huang, Kai wrote:
> On Sun, 2023-06-18 at 00:08 +0000, Mingwei Zhang wrote:
> > Add the missing ptep in kvm_mmu_page description. ptep is used when TDP MMU
> > is enabled and it shares the storage with parent_ptes. Update the doc to
> > help readers to get up-to-date info.
> > 
> > Signed-off-by: Mingwei Zhang <mizhang@google.com>
> > ---
> >  Documentation/virt/kvm/x86/mmu.rst | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/Documentation/virt/kvm/x86/mmu.rst b/Documentation/virt/kvm/x86/mmu.rst
> > index 149dd3cba48f..36bfe0fe02bb 100644
> > --- a/Documentation/virt/kvm/x86/mmu.rst
> > +++ b/Documentation/virt/kvm/x86/mmu.rst
> > @@ -236,6 +236,10 @@ Shadow pages contain the following information:
> >      parent_ptes points at this single spte, otherwise, there exists multiple
> >      sptes pointing at this page and (parent_ptes & ~0x1) points at a data
> >      structure with a list of parent sptes.
> > +  ptep:
> > +    Pointer to the parent spte when TDP MMU is enabled. 
> > 
> 
> IMHO "parent spte" alone _may_ be confusing.  I think it's better to explicitly
> mention "pointing to this page" similar to the "parent_ptes" above.

Sure. I can change the style to be consistent with the descriptions of
'parent_ptes'.

> 
> Also, I think "when TDP MMU is enabled" isn't strictly true, depending on what
> does "when TDP MMU is enabled mean".  E.g., when tdp_mmu_enabled module
> parameter is true, we can still have a nested EPT shadow page from L2 which
> won't use this either IIUC.
> 
hmm, "when TDP MMU is enabled" should be "when used by TDP MMU". You are
right since when TDP MMU is used for L1, we may still have shadow MMUs
for L2s. I modify the description.

> > In TDP MMU, each
> > +    shadow page will have at most one parent. Note that this field is a
> > +    union with parent_ptes.
> 
> Also, perhaps "have at most one parent" can be more precise: only root page has
> no parent, while other non-root pages always have one parent SPTE pointing to
> each of them.

Will do in next version.
diff mbox series

Patch

diff --git a/Documentation/virt/kvm/x86/mmu.rst b/Documentation/virt/kvm/x86/mmu.rst
index 149dd3cba48f..36bfe0fe02bb 100644
--- a/Documentation/virt/kvm/x86/mmu.rst
+++ b/Documentation/virt/kvm/x86/mmu.rst
@@ -236,6 +236,10 @@  Shadow pages contain the following information:
     parent_ptes points at this single spte, otherwise, there exists multiple
     sptes pointing at this page and (parent_ptes & ~0x1) points at a data
     structure with a list of parent sptes.
+  ptep:
+    Pointer to the parent spte when TDP MMU is enabled. In TDP MMU, each
+    shadow page will have at most one parent. Note that this field is a
+    union with parent_ptes.
   unsync:
     If true, then the translations in this page may not match the guest's
     translation.  This is equivalent to the state of the tlb when a pte is