> -----Original Message----- > From: Oliver Upton [mailto:oliver.upton@xxxxxxxxx] > Sent: 15 September 2023 23:54 > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@xxxxxxxxxx> > Cc: kvmarm@xxxxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx; > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; maz@xxxxxxxxxx; will@xxxxxxxxxx; > catalin.marinas@xxxxxxx; james.morse@xxxxxxx; > suzuki.poulose@xxxxxxx; yuzenghui <yuzenghui@xxxxxxxxxx>; zhukeqian > <zhukeqian1@xxxxxxxxxx>; Jonathan Cameron > <jonathan.cameron@xxxxxxxxxx>; Linuxarm <linuxarm@xxxxxxxxxx> > Subject: Re: [RFC PATCH v2 4/8] KVM: arm64: Set DBM for previously > writeable pages > > On Fri, Aug 25, 2023 at 10:35:24AM +0100, Shameer Kolothum wrote: > > We only set DBM if the page is writeable (S2AP[1] == 1). But once > migration > > starts, CLEAR_LOG path will write protect the pages (S2AP[1] = 0) and > there > > isn't an easy way to differentiate the writeable pages that gets write > > protected from read-only pages as we only have S2AP[1] bit to check. > > > > Introduced a ctx->flag KVM_PGTABLE_WALK_WC_HINT to identify the > dirty page > > tracking related write-protect page table walk and used one of the > "Reserved > > for software use" bit in page descriptor to mark a page as > "writeable-clean". > > > > Signed-off-by: Shameer Kolothum > <shameerali.kolothum.thodi@xxxxxxxxxx> > > --- > > arch/arm64/include/asm/kvm_pgtable.h | 5 +++++ > > arch/arm64/kvm/hyp/pgtable.c | 25 > ++++++++++++++++++++++--- > > 2 files changed, 27 insertions(+), 3 deletions(-) > > > > diff --git a/arch/arm64/include/asm/kvm_pgtable.h > b/arch/arm64/include/asm/kvm_pgtable.h > > index a12add002b89..67bcbc5984f9 100644 > > --- a/arch/arm64/include/asm/kvm_pgtable.h > > +++ b/arch/arm64/include/asm/kvm_pgtable.h > > @@ -190,6 +190,8 @@ enum kvm_pgtable_prot { > > #define KVM_PGTABLE_PROT_RW (KVM_PGTABLE_PROT_R | > KVM_PGTABLE_PROT_W) > > #define KVM_PGTABLE_PROT_RWX (KVM_PGTABLE_PROT_RW | > KVM_PGTABLE_PROT_X) > > > > +#define KVM_PGTABLE_PROT_WC KVM_PGTABLE_PROT_SW0 > /*write-clean*/ > > + > > #define PKVM_HOST_MEM_PROT KVM_PGTABLE_PROT_RWX > > #define PKVM_HOST_MMIO_PROT KVM_PGTABLE_PROT_RW > > > > @@ -221,6 +223,8 @@ typedef bool (*kvm_pgtable_force_pte_cb_t)(u64 > addr, u64 end, > > * operations required. > > * @KVM_PGTABLE_WALK_HW_DBM: Indicates that the attribute > update is > > * HW DBM related. > > + * @KVM_PGTABLE_WALK_WC_HINT: Update the page as > writeable-clean(software attribute) > > + * if we are write protecting a writeable page. > > This really looks like a permission bit, not a walker flag. This should > be defined in kvm_pgtable_prot and converted to the hardware definition > in stage2_set_prot_attr(). Also, the first time I saw 'WC' I read it as > 'write-combine', not writable-clean. Ok. I will have a go as suggested above. In fact "writeable-clean" is an ARM ARM term, I will make it more specific. > > As I understand it, the only need for an additional software bit here is > to identify neighboring PTEs that can have DBM set while we're in the > middle of the walk right? Yes, that is the purpose. Thanks, Shameer
