[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] xen/arm: Add support for 16 bit VMIDs



On 23/11/16 10:38, Bhupinder Thakur wrote:
Hi,

Hi Bhupinder,

Can you configure your e-mail client to use ">" to quote the previous answer? Using tab makes the mail quite confusing. Thank you.


         static DECLARE_BITMAP(vmid_mask, MAX_VMID);

         void p2m_vmid_allocator_init(void)
         {
        +    unsigned int cpu;
        +
             set_bit(INVALID_VMID, vmid_mask);
        +
        +    max_vmid = MAX_VMID;


    max_vmid is only declared for ARM64 and will break compilation for
    ARM32. Please try to build each patch for both architecture before
    sending to the ML.

I will compile for ARM32.


    However, in this case. It would make more sense to keep the maximum
    vmid static for ARM32 as it will never change.


    I quite like what has been done for P2M_ROOT_{LEVEL,ORDER} where the
    define is a constant on ARM32 and point to a variable on ARM64.

I will keep the bitmap statically defined for ARM32. For ARM64 bitmap
will be allocated dynamically.

I am not asking the bitmap to be static but the number of vmid to be a constant (not a variable) for ARM32.

I prefer to have the bitmap dynamic on both architecture because it is will be easier to maintain the code in long term.

What I was asking for is:

#ifdef CONFIG_ARM_64
/* The VMID is encoded on 8-bit minimum. */
static unsigned int __read_mostly max_vmid = MAX_VMID_8BIT;
#define MAX_VMID max_vmid
#else
/* The VMID is always encoded on 8-bit */
#define MAX_VMID MAX_VMID_8BIT
#endif

The code would use MAX_VMID and the variable max_vmid would only be defined for arm64 breaking the compilation directly if used in the common code.

[...]

         #endif

         #define VTCR_RES1       (_AC(1,UL)<<31)
        @@ -269,6 +270,11 @@
         /* FSR long format */
         #define FSRL_STATUS_DEBUG       (_AC(0x22,UL)<<0)

        +#ifdef CONFIG_ARM_64
        +#define VMID_8_BITS_SUPPORT         0x0
        +#define VMID_16_BITS_SUPPORT        0x2
        +#endif


    I would prefix the 2 define with MM64_ so we know how the values
    will be matched.

You mean I define them like this ?
#define MM64_VMID_8_BITS_SUPPORT   0x0
#define MM64_VMID_16_BITS_SUPPORT 0x2

Yes, or any other name mentioning the register name in the define. This is how are defined all the register constant within this file.

Regards,

--
Julien Grall

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.