On 13/05/2018 21:08, Liran Alon wrote:
>>
>> +/* Bits which may be returned by set_spte() */
>> +#define WRPROT_SHADOW_PT BIT(0)
> I would rename it to MUST_WRITE_PROTECT. It is true shadow PTE is
> write-protected only in case PFN points to a guest page-table
> which cannot be unsynced but I think this is more clear.
It has really been write protected already by the time the function
returns, hence my suggestion of SET_SPTE_WRITE_PROTECTED.
Paolo
>
>> +#define NEED_FLUSH_REMOTE_TLBS BIT(1)
>> +
>> static int set_spte(struct kvm_vcpu *vcpu, u64 *sptep,
>> unsigned pte_access, int level,
>> gfn_t gfn, kvm_pfn_t pfn, bool speculative,
>> @@ -2801,7 +2805,7 @@ static int set_spte(struct kvm_vcpu *vcpu, u64
>> *sptep,
>> if (mmu_need_write_protect(vcpu, gfn, can_unsync)) {
>> pgprintk("%s: found shadow page for %llx, marking ro\n",
>> __func__, gfn);
>> - ret = 1;
>> + ret |= WRPROT_SHADOW_PT;
>> pte_access &= ~ACC_WRITE_MASK;
>> spte &= ~(PT_WRITABLE_MASK | SPTE_MMU_WRITEABLE);
>> }
>> @@ -2817,7 +2821,7 @@ static int set_spte(struct kvm_vcpu *vcpu, u64
>> *sptep,
>>
>> set_pte:
>> if (mmu_spte_update(sptep, spte))
>> - kvm_flush_remote_tlbs(vcpu->kvm);
>> + ret |= NEED_FLUSH_REMOTE_TLBS;
>> done:
>> return ret;
>> }
>> @@ -2828,6 +2832,7 @@ static int mmu_set_spte(struct kvm_vcpu *vcpu,
>> u64 *sptep, unsigned pte_access,
>> {
>> int was_rmapped = 0;
>> int rmap_count;
>> + int set_spte_ret;
>> int ret = RET_PF_RETRY;
>>
>> pgprintk("%s: spte %llx write_fault %d gfn %llx\n", __func__,
>> @@ -2855,12 +2860,15 @@ static int mmu_set_spte(struct kvm_vcpu *vcpu,
>> u64 *sptep, unsigned pte_access,
>> was_rmapped = 1;
>> }
>>
>> - if (set_spte(vcpu, sptep, pte_access, level, gfn, pfn, speculative,
>> - true, host_writable)) {
>> + set_spte_ret = set_spte(vcpu, sptep, pte_access, level, gfn, pfn,
>> + speculative, true, host_writable);
>> + if (set_spte_ret & WRPROT_SHADOW_PT) {
>> if (write_fault)
>> ret = RET_PF_EMULATE;
>> kvm_make_request(KVM_REQ_TLB_FLUSH, vcpu);
>> }
>> + if (set_spte_ret & NEED_FLUSH_REMOTE_TLBS)
>> + kvm_flush_remote_tlbs(vcpu->kvm);
>>
>> if (unlikely(is_mmio_spte(*sptep)))
>> ret = RET_PF_EMULATE;
>> diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h
>> index 6288e9d7068e..f176b85767ec 100644
>> --- a/arch/x86/kvm/paging_tmpl.h
>> +++ b/arch/x86/kvm/paging_tmpl.h
