2017-12-18 12:55:40

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [PATCH v5 1/2] x86/mm: add a function to check if a pfn is UC/UC-

On 08/11/2017 08:56, Haozhong Zhang wrote:
> +bool pat_pfn_is_uc_or_uc_minus(unsigned long pfn)
> +{
> + enum page_cache_mode cm = lookup_memtype(PFN_PHYS(pfn));
> +
> + return cm == _PAGE_CACHE_MODE_UC || cm == _PAGE_CACHE_MODE_UC_MINUS;
> +}
> +EXPORT_SYMBOL_GPL(pat_pfn_is_uc_or_uc_minus);
> +

As discussed in the reply to v2, this should include WC too. The
function name could become something like pat_pfn_downgraded_by_uc_mtrr.

Thanks,

Paolo


2017-12-19 02:41:15

by Haozhong Zhang

[permalink] [raw]
Subject: Re: [PATCH v5 1/2] x86/mm: add a function to check if a pfn is UC/UC-

On 12/18/17 13:55 +0100, Paolo Bonzini wrote:
> On 08/11/2017 08:56, Haozhong Zhang wrote:
> > +bool pat_pfn_is_uc_or_uc_minus(unsigned long pfn)
> > +{
> > + enum page_cache_mode cm = lookup_memtype(PFN_PHYS(pfn));
> > +
> > + return cm == _PAGE_CACHE_MODE_UC || cm == _PAGE_CACHE_MODE_UC_MINUS;
> > +}
> > +EXPORT_SYMBOL_GPL(pat_pfn_is_uc_or_uc_minus);
> > +
>
> As discussed in the reply to v2, this should include WC too. The
> function name could become something like pat_pfn_downgraded_by_uc_mtrr.

Or shall we just expose lookup_memtype(), and keep all other logic in
KVM? The function name still looks strange somehow, and the check of
memory type makes more sense and would be easier to understand in the
context of KVM.

Haozhong

2017-12-19 10:42:37

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [PATCH v5 1/2] x86/mm: add a function to check if a pfn is UC/UC-

On 19/12/2017 03:40, Haozhong Zhang wrote:
>> As discussed in the reply to v2, this should include WC too. The
>> function name could become something like pat_pfn_downgraded_by_uc_mtrr.
>
> Or shall we just expose lookup_memtype(), and keep all other logic in
> KVM? The function name still looks strange somehow, and the check of
> memory type makes more sense and would be easier to understand in the
> context of KVM.

I agree that it's a bit strange, but I don't want to go against the
suggestions of the x86 maintainer.

Paolo