Re: [Xen-devel] [PATCH v3.1 08/15] x86/vtd: fix mapping of RMRR regions

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 29.10.16 at 10:59, <roger.pau@xxxxxxxxxx> wrote:
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -1049,22 +1049,29 @@ int set_identity_p2m_entry(struct domain *d, unsigned 
> long gfn,
>  
>      mfn = p2m->get_entry(p2m, gfn, &p2mt, &a, 0, NULL, NULL);
>  
> -    if ( p2mt == p2m_invalid || p2mt == p2m_mmio_dm )
> +    switch ( p2mt )
> +    {
> +    case p2m_invalid:
> +    case p2m_mmio_dm:
>          ret = p2m_set_entry(p2m, gfn, _mfn(gfn), PAGE_ORDER_4K,
>                              p2m_mmio_direct, p2ma);
> -    else if ( mfn_x(mfn) == gfn && p2mt == p2m_mmio_direct && a == p2ma )
> -    {
> -        ret = 0;
> -        /*
> -         * PVH fixme: during Dom0 PVH construction, p2m entries are being set
> -         * but iomem regions are not mapped with IOMMU. This makes sure that
> -         * RMRRs are correctly mapped with IOMMU.
> -         */
> -        if ( is_hardware_domain(d) && !iommu_use_hap_pt(d) )
> +        if ( ret )
> +            break;
> +        /* fallthrough */
> +    case p2m_mmio_direct:
> +        if ( p2mt == p2m_mmio_direct && a != p2ma )

I don't understand the removal of the MFN == GFN check, and it
also isn't being explained in the commit message.

And then following a case label with a comparison of the respective
switch expression against the very value from the case label is
certainly odd. I'm pretty sure a better structure of the code could be
found.

> +        {
> +            printk(XENLOG_G_WARNING
> +                   "Cannot setup identity map d%d:%lx, already mapped with "
> +                   "different access type (current: %d, requested: %d).\n",

Please avoid full stops at the end of log messages.

Jan


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