On Thu, Jun 23, 2022 at 6:12 AM Quentin Perret <qperret@xxxxxxxxxx> wrote: > > Hi Peter, > > On Wednesday 22 Jun 2022 at 19:19:24 (-0700), Peter Collingbourne wrote: > > @@ -677,9 +678,9 @@ static bool stage2_pte_is_counted(kvm_pte_t pte) > > /* > > * The refcount tracks valid entries as well as invalid entries if they > > * encode ownership of a page to another entity than the page-table > > - * owner, whose id is 0. > > + * owner, whose id is 0, or NOBODY, which does not correspond to a page-table. > > */ > > - return !!pte; > > + return !!pte && pte != kvm_init_invalid_leaf_owner(PKVM_ID_NOBODY); > > } > > I'm not sure to understand this part? By not refcounting the PTEs that > are annotated with PKVM_ID_NOBODY, the page-table page that contains > them may be freed at some point. And when that happens, I don't see how > the hypervisor will remember to block host accesses to the disowned > pages. This was because I misunderstood the code and thought that this was for maintaining a count from PTEs to the pages that they reference (which would make the refcounting unnecessary for pages owned by nobody). Reading the code more carefully my understanding is now that the refcounts are instead for tracking the number of non-zero PTEs in the page table, so that the hypervisor knows that it can free up a page table when it becomes all zeros i.e. no longer contains any information that needs to be preserved (so the "references" that are being counted are implicit in the location of the PTE). So it doesn't make sense to not track PTEs owned by nobody here. I'll remove this part in v2. Peter
