Re: [Xen-devel] [PATCH v2 05/10] libxl/xl: add memory policy option to iomem

[Stefano Stabellini <sstabellini@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2172 bytes --]

On Tue, 18 Jun 2019, Julien Grall wrote:
> Sorry for the formatting.
> 
> On Tue, 18 Jun 2019, 23:09 Stefano Stabellini, <sstabellini@kernel.org> wrote:
>       On Tue, 18 Jun 2019, Julien Grall wrote:
>       > On 30/04/2019 22:02, Stefano Stabellini wrote:
>       > > diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
>       > > index 89fe80f..a6c5e30 100644
>       > > --- a/tools/libxl/libxl_create.c
>       > > +++ b/tools/libxl/libxl_create.c
>       > > @@ -415,6 +415,21 @@ static void init_console_info(libxl__gc *gc,
>       > >          Only 'channels' when mapped to consoles have a string name. */
>       > >   }
>       > >   +static uint32_t libxl__memory_policy_to_xc(libxl_memory_policy c)
>       > > +{
>       > > +    switch (c) {
>       > > +    case LIBXL_MEMORY_POLICY_ARM_MEM_WB:
>       > > +        return MEMORY_POLICY_ARM_MEM_WB;
>       > > +    case LIBXL_MEMORY_POLICY_ARM_DEV_NGRE:
>       > > +        return MEMORY_POLICY_ARM_DEV_nGRE;
>       > > +    case LIBXL_MEMORY_POLICY_X86_UC:
>       > > +        return MEMORY_POLICY_X86_UC;
>       > > +    case LIBXL_MEMORY_POLICY_DEFAULT:
>       > > +    default:
>       >
>       > Looking at this again, don't we want to bail out if the policy is unknown? My
>       > concern here is the user may configure with something it didn't expect. The
>       > risk is the problem will be hard to debug.
>       >
>       > I also believe this could be part of libxl_{arm,x86}.c allowing us to filter
>       > misuse early.
> 
>       This sounds like a good idea, I can do that. Then, I can also #ifdef the
>       hypercalls defines, although for some reason today libxl doesn't have
>       CONFIG_X86 or CONFIG_ARM set so I would also have to do the following in
>       the libxl Makefile:
> 
>       ifeq ($(CONFIG_X86),y)
>       CFLAGS_LIBXL += -DCONFIG_X86
>       else
>       CFLAGS_LIBXL += -DCONFIG_ARM
>       endif
> 
> 
> Or just follow what we do today in other public headers:
> 
> #if defined(__arm__) || defined(__aarch64__)
> 
> You need to double check the exact syntax as I wrote it by memory.

Doh! Thank you

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

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