Re: [Xen-devel] [PATCH v3 2/5] xen: extend XEN_DOMCTL_memory_mapping to handle memory policy

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 19.06.19 at 01:20, <sstabellini@xxxxxxxxxx> wrote:
> --- a/tools/libxc/xc_domain.c
> +++ b/tools/libxc/xc_domain.c
> @@ -2070,6 +2070,7 @@ int xc_domain_memory_mapping(
>      domctl.cmd = XEN_DOMCTL_memory_mapping;
>      domctl.domain = domid;
>      domctl.u.memory_mapping.add_mapping = add_mapping;
> +    domctl.u.memory_mapping.memory_policy = 0;

Why not MEMORY_POLICY_DEFAULT?

> --- a/xen/common/domctl.c
> +++ b/xen/common/domctl.c
> @@ -928,6 +928,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 
> u_domctl)
>          unsigned long mfn_end = mfn + nr_mfns - 1;
>          int add = op->u.memory_mapping.add_mapping;
>          p2m_type_t p2mt;
> +        uint32_t memory_policy = op->u.memory_mapping.memory_policy;

I can't see the need for a fixed-width type here.

> @@ -958,9 +959,28 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 
> u_domctl)
>          if ( add )
>          {
>              printk(XENLOG_G_DEBUG
> -                   "memory_map:add: dom%d gfn=%lx mfn=%lx nr=%lx\n",
> -                   d->domain_id, gfn, mfn, nr_mfns);
> +                   "memory_map:add: dom%d gfn=%lx mfn=%lx nr=%lx cache=%u\n",
> +                   d->domain_id, gfn, mfn, nr_mfns, memory_policy);

Why "cache=" when it's a "policy" value?

> +            switch ( memory_policy )
> +            {
> +#ifdef CONFIG_ARM
> +                case MEMORY_POLICY_ARM_MEM_WB:
> +                    p2mt = p2m_mmio_direct_c;
> +                    break;
> +                case MEMORY_POLICY_ARM_DEV_nGnRE:
> +                    p2mt = p2m_mmio_direct_dev;
> +                    break;
> +#endif
> +#ifdef CONFIG_X86
> +                case MEMORY_POLICY_X86_UC_MINUS:

FTR - I could certainly live with this becoming MEMORY_POLICY_DEFAULT
for now, if that's really what Andrew prefers for x86.

> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -571,12 +571,33 @@ struct xen_domctl_bind_pt_irq {
>  */
>  #define DPCI_ADD_MAPPING         1
>  #define DPCI_REMOVE_MAPPING      0
> +/*
> + * Default memory policy. Corresponds to:
> + * Arm: MEMORY_POLICY_ARM_DEV_nGnRE
> + * x86: MEMORY_POLICY_X86_UC_MINUS
> + */
> +#define MEMORY_POLICY_DEFAULT         0
> +#if defined(__i386__) || defined(__x86_64__)
> +/* x86 only. Memory type UNCACHABLE */
> +# define MEMORY_POLICY_X86_UC_MINUS    0
> +#elif defined(__arm__) || defined (__aarch64__)
> +/* Arm only. Outer Shareable, Device-nGnRE memory (Device Memory on Armv7) 
> */
> +# define MEMORY_POLICY_ARM_DEV_nGnRE      0
> +/* Arm only. Outer Shareable, Outer/Inner Write-Back Cacheable memory */
> +# define MEMORY_POLICY_ARM_MEM_WB         1
> +/*
> + * On ARM, MEMORY_POLICY selects the stage-2 memory attributes, but note

Further up it's Arm - why all upper case here?

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel