[PATCH 0/6] KVM: Documentation: Update document description for kvm_mmu_page and kvm_mmu_page_role

[PATCH 0/6] KVM: Documentation: Update document description for kvm_mmu_page and kvm_mmu_page_role

[Mingwei Zhang]

When reading the KVM MMU documentation for nested virtualization, I feel
that the description of kvm_mmu_page (and kvm_mmu_page_role) has been
lagging for around 1-2 years. The descriptions for several fields in struct
kvm_mmu_page and struct kvm_mmu_page_role are missing. So I think it might
be good to add them to make it consistent with the current code.

Note that there are still some fields not added in this series:
 - kvm_mmu_page.nx_huge_page_disallowed
 - kvm_mmu_page.possible_nx_huge_page_link
 - kvm_mmu_page.hash_link
 - kvm_mmu_page.link

For the above, I thought the description might be just better to be
inlined or there is already good description inlined.

Mingwei Zhang (6):
  KVM: Documentation: Add the missing guest_mode in kvm_mmu_page_role
  KVM: Documentation: Update the field name gfns in kvm_mmu_page
  KVM: Documentation: Add the missing ptep in kvm_mmu_page
  KVM: Documentation: Add the missing tdp_mmu_root_count into
    kvm_mmu_page
  KVM: Documentation: Add the missing mmu_valid_gen into kvm_mmu_page
  KVM: Documentation: Add the missing tdp_mmu_page into kvm_mmu_page

 Documentation/virt/kvm/x86/mmu.rst | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)


base-commit: 265b97cbc22e0f67f79a71443b60dc1237ca5ee6
-- 
2.41.0.162.gfafddb0af9-goog

[PATCH 1/6] KVM: Documentation: Add the missing guest_mode in kvm_mmu_page_role

[Mingwei Zhang]

Add the missing guest_mode in kvm_mmu_page_role description. guest_mode
tells KVM whether a shadow page is used for the L1 or an L2. Update the
missing field in documentation.

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

diff --git a/Documentation/virt/kvm/x86/mmu.rst b/Documentation/virt/kvm/x86/mmu.rst
index 8364afa228ec..561efa8ec7d7 100644
--- a/Documentation/virt/kvm/x86/mmu.rst
+++ b/Documentation/virt/kvm/x86/mmu.rst
@@ -202,6 +202,8 @@ Shadow pages contain the following information:
     Is 1 if the MMU instance cannot use A/D bits.  EPT did not have A/D
     bits before Haswell; shadow EPT page tables also cannot use A/D bits
     if the L1 hypervisor does not enable them.
+  role.guest_mode:
+    Indicates the shadow page is created for a nested guest.
   role.passthrough:
     The page is not backed by a guest page table, but its first entry
     points to one.  This is set if NPT uses 5-level page tables (host
-- 
2.41.0.162.gfafddb0af9-goog

[PATCH 2/6] KVM: Documentation: Update the field name gfns in kvm_mmu_page

[Mingwei Zhang]

Update the 'gfns' in kvm_mmu_page to 'shadowed_translation'to be consistent
with the code. The more detailed description of 'shadowed_translation' is
already inlined in the data structure definition, so no need to duplicate
the text but simply just update the name.

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

diff --git a/Documentation/virt/kvm/x86/mmu.rst b/Documentation/virt/kvm/x86/mmu.rst
index 561efa8ec7d7..149dd3cba48f 100644
--- a/Documentation/virt/kvm/x86/mmu.rst
+++ b/Documentation/virt/kvm/x86/mmu.rst
@@ -221,7 +221,7 @@ Shadow pages contain the following information:
     at __pa(sp2->spt).  sp2 will point back at sp1 through parent_pte.
     The spt array forms a DAG structure with the shadow page as a node, and
     guest pages as leaves.
-  gfns:
+  shadowed_translation:
     An array of 512 guest frame numbers, one for each present pte.  Used to
     perform a reverse map from a pte to a gfn. When role.direct is set, any
     element of this array can be calculated from the gfn field when used, in
-- 
2.41.0.162.gfafddb0af9-goog

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

[Mingwei Zhang]

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. 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
-- 
2.41.0.162.gfafddb0af9-goog

[PATCH 4/6] KVM: Documentation: Add the missing tdp_mmu_root_count into kvm_mmu_page

[Mingwei Zhang]

Add tdp_mmu_root_count into kvm_mmu_page description. tdp_mmu_root_count is
an atomic counter used only in TDP MMU. Its usage and meaning is slightly
different with root_counter in shadow MMU. Update the doc.

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

diff --git a/Documentation/virt/kvm/x86/mmu.rst b/Documentation/virt/kvm/x86/mmu.rst
index 36bfe0fe02bb..3dce2426ad6d 100644
--- a/Documentation/virt/kvm/x86/mmu.rst
+++ b/Documentation/virt/kvm/x86/mmu.rst
@@ -230,6 +230,12 @@ Shadow pages contain the following information:
     A counter keeping track of how many hardware registers (guest cr3 or
     pdptrs) are now pointing at the page.  While this counter is nonzero, the
     page cannot be destroyed.  See role.invalid.
+  tdp_mmu_root_count:
+    An atomic reference counter in TDP MMU that allows for parallel
+    accesses.  Accessing the page requires lifting the counter value. The
+    initial value is set as 2 indicating one reference from vCPU and one
+    from TDP MMU itself. While this counter is nonzero, the page cannot be
+    destroyed. Note this field is a union with root_count.
   parent_ptes:
     The reverse mapping for the pte/ptes pointing at this page's spt. If
     parent_ptes bit 0 is zero, only one spte points at this page and
-- 
2.41.0.162.gfafddb0af9-goog

[PATCH 5/6] KVM: Documentation: Add the missing mmu_valid_gen into kvm_mmu_page

[Mingwei Zhang]

Add mmu_valid_gen into kvm_mmu_page description. mmu_valid_gen is used in
shadow MMU for fast zapping. Update the doc to reflect that.

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 3dce2426ad6d..0dbdb7fb8cc6 100644
--- a/Documentation/virt/kvm/x86/mmu.rst
+++ b/Documentation/virt/kvm/x86/mmu.rst
@@ -208,6 +208,10 @@ Shadow pages contain the following information:
     The page is not backed by a guest page table, but its first entry
     points to one.  This is set if NPT uses 5-level page tables (host
     CR4.LA57=1) and is shadowing L1's 4-level NPT (L1 CR4.LA57=1).
+  mmu_valid_gen:
+    Used by comparing against kvm->arch.mmu_valid_gen to check whether the
+    shadow page is obsolete thus a convenient variable for fast zapping.
+    Note that TDP MMU does not use mmu_valid_gen.
   gfn:
     Either the guest page table containing the translations shadowed by this
     page, or the base page frame for linear translations.  See role.direct.
-- 
2.41.0.162.gfafddb0af9-goog

[PATCH 6/6] KVM: Documentation: Add the missing tdp_mmu_page into kvm_mmu_page

[Mingwei Zhang]

Add tdp_mmu_page into kvm_mmu_page description. tdp_mmu_page is a field to
differentiate shadow pages from TDP MMU and non-TDP MMU. When TDP MMU is
enabled, sp->tdp_mmu_page=1 indicates a shadow page for L1, while
sp->tdp_mmu_page=0 indicates a shadow page for an L2. When TDP MMU is
disabled, sp->tdp_mmu_page is always 0. So update the doc to reflect the
information.

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 0dbdb7fb8cc6..cbad49c37629 100644
--- a/Documentation/virt/kvm/x86/mmu.rst
+++ b/Documentation/virt/kvm/x86/mmu.rst
@@ -277,6 +277,10 @@ Shadow pages contain the following information:
     since the last time the page table was actually used; if emulation
     is triggered too frequently on this page, KVM will unmap the page
     to avoid emulation in the future.
+  tdp_mmu_page:
+    Is 1 if the shadow page is a TDP MMU page. When TDP MMU is disabled,
+    this field is always 0.
+
 
 Reverse map
 ===========
-- 
2.41.0.162.gfafddb0af9-goog

Re: [PATCH 0/6] KVM: Documentation: Update document description for kvm_mmu_page and kvm_mmu_page_role

[Zhi Wang]

On Sun, 18 Jun 2023 00:08:50 +0000
Mingwei Zhang <mizhang@google.com> wrote:

> When reading the KVM MMU documentation for nested virtualization, I feel
> that the description of kvm_mmu_page (and kvm_mmu_page_role) has been
> lagging for around 1-2 years. The descriptions for several fields in struct
> kvm_mmu_page and struct kvm_mmu_page_role are missing. So I think it might
> be good to add them to make it consistent with the current code.
>

This is so nice. A trivial comment, maybe refining the tittle of PATCH 1-6 a
bit: "Add the missing comment of xxxx into xxxx". It is a little bit confusing
by just looking at the titles at the first glance, more like some members are
missing, not the comments. :)

> Note that there are still some fields not added in this series:
>  - kvm_mmu_page.nx_huge_page_disallowed
>  - kvm_mmu_page.possible_nx_huge_page_link
>  - kvm_mmu_page.hash_link
>  - kvm_mmu_page.link
> 
> For the above, I thought the description might be just better to be
> inlined or there is already good description inlined.
> 
> Mingwei Zhang (6):
>   KVM: Documentation: Add the missing guest_mode in kvm_mmu_page_role
>   KVM: Documentation: Update the field name gfns in kvm_mmu_page
>   KVM: Documentation: Add the missing ptep in kvm_mmu_page
>   KVM: Documentation: Add the missing tdp_mmu_root_count into
>     kvm_mmu_page
>   KVM: Documentation: Add the missing mmu_valid_gen into kvm_mmu_page
>   KVM: Documentation: Add the missing tdp_mmu_page into kvm_mmu_page
> 
>  Documentation/virt/kvm/x86/mmu.rst | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> 
> base-commit: 265b97cbc22e0f67f79a71443b60dc1237ca5ee6

Re: [PATCH 0/6] KVM: Documentation: Update document description for kvm_mmu_page and kvm_mmu_page_role

[Mingwei Zhang]

On Mon, Jun 19, 2023 at 2:00 AM Zhi Wang <zhi.wang.linux@gmail.com> wrote:
>
> On Sun, 18 Jun 2023 00:08:50 +0000
> Mingwei Zhang <mizhang@google.com> wrote:
>
> > When reading the KVM MMU documentation for nested virtualization, I feel
> > that the description of kvm_mmu_page (and kvm_mmu_page_role) has been
> > lagging for around 1-2 years. The descriptions for several fields in struct
> > kvm_mmu_page and struct kvm_mmu_page_role are missing. So I think it might
> > be good to add them to make it consistent with the current code.
> >
>
> This is so nice. A trivial comment, maybe refining the tittle of PATCH 1-6 a
> bit: "Add the missing comment of xxxx into xxxx". It is a little bit confusing
> by just looking at the titles at the first glance, more like some members are
> missing, not the comments. :)
>

Sure. Yeah, the shortlog looks slightly confusing as if I am changing
the actual data structures. I will update those in the next version.
Thanks for your comments.

-Mingwei

Re: [PATCH 1/6] KVM: Documentation: Add the missing guest_mode in kvm_mmu_page_role

[Huang, Kai]

On Sun, 2023-06-18 at 00:08 +0000, Mingwei Zhang wrote:
> Add the missing guest_mode in kvm_mmu_page_role description. guest_mode
> tells KVM whether a shadow page is used for the L1 or an L2. Update the
> missing field in documentation.
> 
> Signed-off-by: Mingwei Zhang <mizhang@google.com>
> ---
>  Documentation/virt/kvm/x86/mmu.rst | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/virt/kvm/x86/mmu.rst b/Documentation/virt/kvm/x86/mmu.rst
> index 8364afa228ec..561efa8ec7d7 100644
> --- a/Documentation/virt/kvm/x86/mmu.rst
> +++ b/Documentation/virt/kvm/x86/mmu.rst
> @@ -202,6 +202,8 @@ Shadow pages contain the following information:
>      Is 1 if the MMU instance cannot use A/D bits.  EPT did not have A/D
>      bits before Haswell; shadow EPT page tables also cannot use A/D bits
>      if the L1 hypervisor does not enable them.
> +  role.guest_mode:
> +    Indicates the shadow page is created for a nested guest.
>    role.passthrough:
>      The page is not backed by a guest page table, but its first entry
>      points to one.  This is set if NPT uses 5-level page tables (host

Reviewed-by: Kai Huang <kai.huang@intel.com>

Re: [PATCH 2/6] KVM: Documentation: Update the field name gfns in kvm_mmu_page

[Huang, Kai]

On Sun, 2023-06-18 at 00:08 +0000, Mingwei Zhang wrote:
> Update the 'gfns' in kvm_mmu_page to 'shadowed_translation'to be consistent
> with the code. The more detailed description of 'shadowed_translation' is
> already inlined in the data structure definition, so no need to duplicate
> the text but simply just update the name.
> 
> Signed-off-by: Mingwei Zhang <mizhang@google.com>
> ---
>  Documentation/virt/kvm/x86/mmu.rst | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/virt/kvm/x86/mmu.rst b/Documentation/virt/kvm/x86/mmu.rst
> index 561efa8ec7d7..149dd3cba48f 100644
> --- a/Documentation/virt/kvm/x86/mmu.rst
> +++ b/Documentation/virt/kvm/x86/mmu.rst
> @@ -221,7 +221,7 @@ Shadow pages contain the following information:
>      at __pa(sp2->spt).  sp2 will point back at sp1 through parent_pte.
>      The spt array forms a DAG structure with the shadow page as a node, and
>      guest pages as leaves.
> -  gfns:
> +  shadowed_translation:
>      An array of 512 guest frame numbers, one for each present pte.  Used to
>      perform a reverse map from a pte to a gfn. When role.direct is set, any
>      element of this array can be calculated from the gfn field when used, in

Reviewed-by: Kai Huang <kai.huang@intel.com>

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

[Huang, Kai]

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.

Re: [PATCH 4/6] KVM: Documentation: Add the missing tdp_mmu_root_count into kvm_mmu_page

[Huang, Kai]

On Sun, 2023-06-18 at 00:08 +0000, Mingwei Zhang wrote:
> Add tdp_mmu_root_count into kvm_mmu_page description. tdp_mmu_root_count is
> an atomic counter used only in TDP MMU. Its usage and meaning is slightly
> different with root_counter in shadow MMU. Update the doc.
> 
> Signed-off-by: Mingwei Zhang <mizhang@google.com>
> ---
>  Documentation/virt/kvm/x86/mmu.rst | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/virt/kvm/x86/mmu.rst b/Documentation/virt/kvm/x86/mmu.rst
> index 36bfe0fe02bb..3dce2426ad6d 100644
> --- a/Documentation/virt/kvm/x86/mmu.rst
> +++ b/Documentation/virt/kvm/x86/mmu.rst
> @@ -230,6 +230,12 @@ Shadow pages contain the following information:
>      A counter keeping track of how many hardware registers (guest cr3 or
>      pdptrs) are now pointing at the page.  While this counter is nonzero, the
>      page cannot be destroyed.  See role.invalid.
> +  tdp_mmu_root_count:
> +    An atomic reference counter in TDP MMU that allows for parallel
					     ^
Please explicitly say "TDP MMU root page" ?
			
> +    accesses.  Accessing the page requires lifting the counter value. The
> +    initial value is set as 2 indicating one reference from vCPU and one
			    ^
			    to ?

> +    from TDP MMU itself. 
> 


> While this counter is nonzero, the page cannot be
> +    destroyed. 
> 

I guess this sentence is implied by "reference counter" mentioned in the first
sentence.

> Note this field is a union with root_count.
> 

Re: [PATCH 6/6] KVM: Documentation: Add the missing tdp_mmu_page into kvm_mmu_page

[Huang, Kai]

On Sun, 2023-06-18 at 00:08 +0000, Mingwei Zhang wrote:
> Add tdp_mmu_page into kvm_mmu_page description. tdp_mmu_page is a field to
> differentiate shadow pages from TDP MMU and non-TDP MMU. When TDP MMU is
> enabled, sp->tdp_mmu_page=1 indicates a shadow page for L1, while
> sp->tdp_mmu_page=0 indicates a shadow page for an L2. When TDP MMU is
> disabled, sp->tdp_mmu_page is always 0. So update the doc to reflect the
> information.
> 
> 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 0dbdb7fb8cc6..cbad49c37629 100644
> --- a/Documentation/virt/kvm/x86/mmu.rst
> +++ b/Documentation/virt/kvm/x86/mmu.rst
> @@ -277,6 +277,10 @@ Shadow pages contain the following information:
>      since the last time the page table was actually used; if emulation
>      is triggered too frequently on this page, KVM will unmap the page
>      to avoid emulation in the future.
> +  tdp_mmu_page:
> +    Is 1 if the shadow page is a TDP MMU page. When TDP MMU is disabled,
> +    this field is always 0.
> +
> 

Hmm.. Again I think "TDP MMU is disabled" is a little bit confusing, but maybe
it's only me having this impression.

I think you can just delete the second sentence.

Re: [PATCH 5/6] KVM: Documentation: Add the missing mmu_valid_gen into kvm_mmu_page

[Huang, Kai]

On Sun, 2023-06-18 at 00:08 +0000, Mingwei Zhang wrote:
> Add mmu_valid_gen into kvm_mmu_page description. mmu_valid_gen is used in
> shadow MMU for fast zapping. Update the doc to reflect that.
> 
> 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 3dce2426ad6d..0dbdb7fb8cc6 100644
> --- a/Documentation/virt/kvm/x86/mmu.rst
> +++ b/Documentation/virt/kvm/x86/mmu.rst
> @@ -208,6 +208,10 @@ Shadow pages contain the following information:
>      The page is not backed by a guest page table, but its first entry
>      points to one.  This is set if NPT uses 5-level page tables (host
>      CR4.LA57=1) and is shadowing L1's 4-level NPT (L1 CR4.LA57=1).
> +  mmu_valid_gen:
> +    Used by comparing against kvm->arch.mmu_valid_gen to check whether the
> +    shadow page is obsolete thus a convenient variable for fast zapping.
> +    Note that TDP MMU does not use mmu_valid_gen.
>    gfn:
>      Either the guest page table containing the translations shadowed by this
>      page, or the base page frame for linear translations.  See role.direct.

Reviewed-by: Kai Huang <kai.huang@intel.com>

Re: [PATCH 2/6] KVM: Documentation: Update the field name gfns in kvm_mmu_page

[Xu Yilun]

On 2023-06-18 at 00:08:52 +0000, Mingwei Zhang wrote:
> Update the 'gfns' in kvm_mmu_page to 'shadowed_translation'to be consistent
> with the code. The more detailed description of 'shadowed_translation' is
> already inlined in the data structure definition, so no need to duplicate
> the text but simply just update the name.

The definition of this field is changed, but apprently the description
here is for gfns. It leaves some confusion if we just leave them
unchanged.

> 
> Signed-off-by: Mingwei Zhang <mizhang@google.com>
> ---
>  Documentation/virt/kvm/x86/mmu.rst | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/virt/kvm/x86/mmu.rst b/Documentation/virt/kvm/x86/mmu.rst
> index 561efa8ec7d7..149dd3cba48f 100644
> --- a/Documentation/virt/kvm/x86/mmu.rst
> +++ b/Documentation/virt/kvm/x86/mmu.rst
> @@ -221,7 +221,7 @@ Shadow pages contain the following information:
>      at __pa(sp2->spt).  sp2 will point back at sp1 through parent_pte.
>      The spt array forms a DAG structure with the shadow page as a node, and
>      guest pages as leaves.
> -  gfns:
> +  shadowed_translation:
>      An array of 512 guest frame numbers, one for each present pte.  Used to

guest frame numbers -> shadow translation info (gfn + access)

>      perform a reverse map from a pte to a gfn. When role.direct is set, any

Just "perform reverse mapping" is OK?

>      element of this array can be calculated from the gfn field when used, in

May remove the "of gfns"

Thanks,
Yilun

> -- 
> 2.41.0.162.gfafddb0af9-goog
> 

Re: [PATCH 2/6] KVM: Documentation: Update the field name gfns in kvm_mmu_page

[Xu Yilun]

On 2023-06-23 at 08:30:54 +0800, Xu Yilun wrote:
> On 2023-06-18 at 00:08:52 +0000, Mingwei Zhang wrote:
> > Update the 'gfns' in kvm_mmu_page to 'shadowed_translation'to be consistent
> > with the code. The more detailed description of 'shadowed_translation' is
> > already inlined in the data structure definition, so no need to duplicate
> > the text but simply just update the name.
> 
> The definition of this field is changed, but apprently the description
> here is for gfns. It leaves some confusion if we just leave them
> unchanged.
> 
> > 
> > Signed-off-by: Mingwei Zhang <mizhang@google.com>
> > ---
> >  Documentation/virt/kvm/x86/mmu.rst | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/virt/kvm/x86/mmu.rst b/Documentation/virt/kvm/x86/mmu.rst
> > index 561efa8ec7d7..149dd3cba48f 100644
> > --- a/Documentation/virt/kvm/x86/mmu.rst
> > +++ b/Documentation/virt/kvm/x86/mmu.rst
> > @@ -221,7 +221,7 @@ Shadow pages contain the following information:
> >      at __pa(sp2->spt).  sp2 will point back at sp1 through parent_pte.
> >      The spt array forms a DAG structure with the shadow page as a node, and
> >      guest pages as leaves.
> > -  gfns:
> > +  shadowed_translation:
> >      An array of 512 guest frame numbers, one for each present pte.  Used to
> 
> guest frame numbers -> shadow translation info (gfn + access)
> 
> >      perform a reverse map from a pte to a gfn. When role.direct is set, any
> 
> Just "perform reverse mapping" is OK?
> 
> >      element of this array can be calculated from the gfn field when used, in
> 
> May remove the "of gfns"

Sorry, forgot to append the original sentence.

    element of this array can be calculated from the gfn field when used, in
    this case, the array of gfns is not allocated. See role.direct and gfn.
                               ^
May remove the "of gfns"

Thanks,
Yilun

> 
> > -- 
> > 2.41.0.162.gfafddb0af9-goog
> > 

Re: [PATCH 2/6] KVM: Documentation: Update the field name gfns in kvm_mmu_page

[Mingwei Zhang]

On Fri, Jun 23, 2023, Xu Yilun wrote:
> On 2023-06-18 at 00:08:52 +0000, Mingwei Zhang wrote:
> > Update the 'gfns' in kvm_mmu_page to 'shadowed_translation'to be consistent
> > with the code. The more detailed description of 'shadowed_translation' is
> > already inlined in the data structure definition, so no need to duplicate
> > the text but simply just update the name.
> 
> The definition of this field is changed, but apprently the description
> here is for gfns. It leaves some confusion if we just leave them
> unchanged.
> 
> > 
> > Signed-off-by: Mingwei Zhang <mizhang@google.com>
> > ---
> >  Documentation/virt/kvm/x86/mmu.rst | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/virt/kvm/x86/mmu.rst b/Documentation/virt/kvm/x86/mmu.rst
> > index 561efa8ec7d7..149dd3cba48f 100644
> > --- a/Documentation/virt/kvm/x86/mmu.rst
> > +++ b/Documentation/virt/kvm/x86/mmu.rst
> > @@ -221,7 +221,7 @@ Shadow pages contain the following information:
> >      at __pa(sp2->spt).  sp2 will point back at sp1 through parent_pte.
> >      The spt array forms a DAG structure with the shadow page as a node, and
> >      guest pages as leaves.
> > -  gfns:
> > +  shadowed_translation:
> >      An array of 512 guest frame numbers, one for each present pte.  Used to
> 
> guest frame numbers -> shadow translation info (gfn + access)

Will add this one. I will avoid the "(gfn + access)" since that is
already described in the comments inline and it may subject to changes,
eg., adding more bits in the future.
> 
> >      perform a reverse map from a pte to a gfn. When role.direct is set, any
> 
> Just "perform reverse mapping" is OK?

I will tend to leave that as is, since it is not a major issue.
> 
> >      element of this array can be calculated from the gfn field when used, in
> 
> May remove the "of gfns"

will do.
>
> Thanks,
> Yilun

Thanks for the comment, will update it in next version.
> 
> > -- 
> > 2.41.0.162.gfafddb0af9-goog
> > 

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

[Mingwei Zhang]

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.

Re: [PATCH 4/6] KVM: Documentation: Add the missing tdp_mmu_root_count into kvm_mmu_page

[Mingwei Zhang]

On Thu, Jun 22, 2023, Huang, Kai wrote:
> On Sun, 2023-06-18 at 00:08 +0000, Mingwei Zhang wrote:
> > Add tdp_mmu_root_count into kvm_mmu_page description. tdp_mmu_root_count is
> > an atomic counter used only in TDP MMU. Its usage and meaning is slightly
> > different with root_counter in shadow MMU. Update the doc.
> > 
> > Signed-off-by: Mingwei Zhang <mizhang@google.com>
> > ---
> >  Documentation/virt/kvm/x86/mmu.rst | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/Documentation/virt/kvm/x86/mmu.rst b/Documentation/virt/kvm/x86/mmu.rst
> > index 36bfe0fe02bb..3dce2426ad6d 100644
> > --- a/Documentation/virt/kvm/x86/mmu.rst
> > +++ b/Documentation/virt/kvm/x86/mmu.rst
> > @@ -230,6 +230,12 @@ Shadow pages contain the following information:
> >      A counter keeping track of how many hardware registers (guest cr3 or
> >      pdptrs) are now pointing at the page.  While this counter is nonzero, the
> >      page cannot be destroyed.  See role.invalid.
> > +  tdp_mmu_root_count:
> > +    An atomic reference counter in TDP MMU that allows for parallel
> 					     ^
> Please explicitly say "TDP MMU root page" ?
>
> > +    accesses.  Accessing the page requires lifting the counter value. The
> > +    initial value is set as 2 indicating one reference from vCPU and one
> 			    ^
> 			    to ?
> 
> > +    from TDP MMU itself. 
> > 
> 
> 
> > While this counter is nonzero, the page cannot be
> > +    destroyed. 
> > 
> 
> I guess this sentence is implied by "reference counter" mentioned in the first
> sentence.
> 

Will make the changes in next version.

-Mingwei

Re: [PATCH 6/6] KVM: Documentation: Add the missing tdp_mmu_page into kvm_mmu_page

[Mingwei Zhang]

On Thu, Jun 22, 2023, Huang, Kai wrote:
> On Sun, 2023-06-18 at 00:08 +0000, Mingwei Zhang wrote:
> > Add tdp_mmu_page into kvm_mmu_page description. tdp_mmu_page is a field to
> > differentiate shadow pages from TDP MMU and non-TDP MMU. When TDP MMU is
> > enabled, sp->tdp_mmu_page=1 indicates a shadow page for L1, while
> > sp->tdp_mmu_page=0 indicates a shadow page for an L2. When TDP MMU is
> > disabled, sp->tdp_mmu_page is always 0. So update the doc to reflect the
> > information.
> > 
> > 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 0dbdb7fb8cc6..cbad49c37629 100644
> > --- a/Documentation/virt/kvm/x86/mmu.rst
> > +++ b/Documentation/virt/kvm/x86/mmu.rst
> > @@ -277,6 +277,10 @@ Shadow pages contain the following information:
> >      since the last time the page table was actually used; if emulation
> >      is triggered too frequently on this page, KVM will unmap the page
> >      to avoid emulation in the future.
> > +  tdp_mmu_page:
> > +    Is 1 if the shadow page is a TDP MMU page. When TDP MMU is disabled,
> > +    this field is always 0.
> > +
> > 
> 
> Hmm.. Again I think "TDP MMU is disabled" is a little bit confusing, but maybe
> it's only me having this impression.
> 
> I think you can just delete the second sentence.

It is not confusing but redundant in this case. I will delete it in next
version.