Re: [Xen-devel] [PATCH 5/6] x86/hvm: Break out __hvm_copy()'s translation logic

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 21.06.17 at 17:12, <andrew.cooper3@xxxxxxxxxx> wrote:
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -3072,6 +3072,80 @@ void hvm_task_switch(
>      hvm_unmap_entry(nptss_desc);
>  }
>  
> +enum hvm_translation_result hvm_translate_get_page(
> +    struct vcpu *v, unsigned long addr, bool linear, uint32_t pfec,
> +    pagefault_info_t *pfinfo, struct page_info **_page,
> +    gfn_t *__gfn, p2m_type_t *_p2mt)

Please avoid leading underscores on parameter names (and even
more so double ones). page_p, gfn_p, and p2mt_p perhaps?

> +{
> +    struct page_info *page;
> +    p2m_type_t p2mt;
> +    gfn_t gfn;
> +
> +    if ( linear )
> +    {
> +        gfn = _gfn(paging_gva_to_gfn(v, addr, &pfec));
> +
> +        if ( gfn_eq(gfn, INVALID_GFN) )
> +        {
> +            if ( pfec & PFEC_page_paged )
> +                return HVMTRANS_gfn_paged_out;
> +
> +            if ( pfec & PFEC_page_shared )
> +                return HVMTRANS_gfn_shared;
> +
> +            if ( pfinfo )
> +            {
> +                pfinfo->linear = addr;
> +                pfinfo->ec = pfec & ~PFEC_implicit;
> +            }
> +
> +            return HVMTRANS_bad_linear_to_gfn;
> +        }
> +    }
> +    else
> +        gfn = _gfn(addr >> PAGE_SHIFT);

As we risk the caller not recognizing that *pfinfo won't be written
to on this "else" path, wouldn't it be a good idea to ASSERT(!pfinfo)
here?

> +    /*
> +     * No need to do the P2M lookup for internally handled MMIO, benefiting
> +     * - 32-bit WinXP (& older Windows) on AMD CPUs for LAPIC accesses,
> +     * - newer Windows (like Server 2012) for HPET accesses.
> +     */
> +    if ( v == current && is_hvm_vcpu(v)

Isn't the is_hvm_vcpu() a stray leftover from the PVHv1 removal,
and hence doesn't need retaining here?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel