Re: [PATCH 3/8] xen/arm: Implement p2m_type_t as an enum

[Julien Grall <julien.grall@linaro.org>]

On 12/05/2013 04:56 PM, Ian Campbell wrote:
> On Thu, 2013-12-05 at 16:44 +0000, Julien Grall wrote:
>>
>> On 12/05/2013 04:38 PM, Ian Campbell wrote:
>>> On Thu, 2013-12-05 at 16:28 +0000, Julien Grall wrote:
>>>>
>>>> On 12/05/2013 04:14 PM, Ian Campbell wrote:
>>>>> On Thu, 2013-12-05 at 16:01 +0000, Julien Grall wrote:
>>>>>>
>>>>>> On 12/05/2013 03:52 PM, Ian Campbell wrote:
>>>>>>> On Thu, 2013-12-05 at 15:42 +0000, Julien Grall wrote:
>>>>>>>> Until now, Xen doesn't know the type of the page (ram, foreign page, mmio,...).
>>>>>>>> Introduce p2m_type_t with basic types:
>>>>>>>>         - p2m_invalid: Nothing is mapped here
>>>>>>>
>>>>>>> Do we really need this? Is it not equivalent to not setting the present
>>>>>>> bit? I see x86 has the same type though -- Tim can you explain why.
>>>>>>
>>>>>> We need a default value when Xen retrieves the p2m type. I don't think
>>>>>> we can assume that p2m_ram_rw (or any other type) is used by default.
>>>>>>
>>>>>>> Since the avail bits in the p2m pte are in pretty short supply I think
>>>>>>> we can avoid unnecessary types.
>>>>>>
>>>>>> I plan to use directly the decimal value. So we can store up to 16 values.
>>>>>
>>>>> 16 is short supply in my book ;-)
>>>>>
>>>>> Having got a bit further through the series I see how p2m_invalid is
>>>>> being used now. It is a useful pseudo-type but it doesn't need to be
>>>>> represented in the avail bits I don't think. How about:
>>>>>
>>>>> typedef enum {
>>>>>        p2m_ram_rw,         /* Normal read/write guest RAM */
>>>>>        p2m_ram_ro,         /* Read-only; writes are silently dropped */
>>>>>        p2m_mmio_direct,    /* Read/write mapping of genuine MMIO area /
>>>>>        p2m_map_foreign,    /* Ram pages from foreign domain */
>>>>>
>>>>>        p2m_max_real_type = 16,    /* Types after this are pseudo-types. */
>>>>>
>>>>>        p2m_invalid,        /* Nothing mapped here */
>>>>>
>>>>> } p2m_type_t;
>>>>>
>>>>> BUILD_BUG_ON(p2m_max_real_type >= 2^4);
>>>>>
>>>>> Now you can return it etc but it never needs to get put in an actual
>>>>> pte?
>>>>
>>>>
>>>> This solution was easier to avoid extra code in the different function.
>>>> I will rework it for the next series.
>>>
>>> "This" is what I suggested here or what you wrote already?
>>
>> The code I wrote.
>
> Maybe we should just keep this trick in our pocket for when we run out
> of bits then?

Sounds good to me. I will add a comment in the code with your solution.

-- 
Julien Grall