Re: [Xen-devel] [PATCH v6 11/29] xen/x86: add bitmap of enabled emulated devices

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 04.09.15 at 14:08, <roger.pau@xxxxxxxxxx> wrote:
> --- a/tools/libxl/libxl_x86.c
> +++ b/tools/libxl/libxl_x86.c
> @@ -7,8 +7,12 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
>                                        libxl_domain_config *d_config,
>                                        xc_domain_configuration_t *xc_config)
>  {
> -    /* No specific configuration right now */
> -
> +    if (d_config->c_info.type == LIBXL_DOMAIN_TYPE_HVM)
> +        xc_config->emulation_flags = (XEN_X86_EMU_LAPIC | XEN_X86_EMU_HPET |
> +                                      XEN_X86_EMU_PMTIMER | XEN_X86_EMU_RTC |
> +                                      XEN_X86_EMU_IOAPIC | XEN_X86_EMU_PIC |
> +                                      XEN_X86_EMU_PMU | XEN_X86_EMU_VGA |
> +                                      XEN_X86_EMU_IOMMU);

This calls for the elsewhere discussed XEN_X86_EMU_ALL to even be
exposed to the tool stack.

> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -555,6 +555,29 @@ int arch_domain_create(struct domain *d, unsigned int 
> domcr_flags,
>                 d->domain_id);
>      }
>  
> +    if ( is_hvm_domain(d) )
> +    {
> +        uint32_t emulation_mask = (XEN_X86_EMU_LAPIC | XEN_X86_EMU_HPET |

const

> +                                   XEN_X86_EMU_PMTIMER | XEN_X86_EMU_RTC |
> +                                   XEN_X86_EMU_IOAPIC | XEN_X86_EMU_PIC |
> +                                   XEN_X86_EMU_PMU | XEN_X86_EMU_VGA |
> +                                   XEN_X86_EMU_IOMMU);
> +        if ( (config->emulation_flags & ~emulation_mask) != 0 )

Missing blank line between declaration and statements.

> +        {
> +            printk(XENLOG_G_ERR "d%d: Invalid emulation bitmap: %#x.\n",

Generally we have no full stops at the end of log messages.

> +                   d->domain_id, config->emulation_flags);
> +            return -EINVAL;
> +        }
> +        if ( config->emulation_flags != emulation_mask )
> +        {
> +            printk(XENLOG_G_ERR "d%d: Xen does not allow HVM creation with 
> the "
> +                   "current selection of emulators: %#x.\n", d->domain_id,
> +                   config->emulation_flags);
> +            return -EOPNOTSUPP;
> +        }
> +        d->arch.emulation_flags = config->emulation_flags;
> +    }

Isn't there an "else" missing here, validating that the flags are zero?

Jan


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