Re: [Xen-devel] [PATCH v2 1/3] xen/domain_page: Convert map_domain_page_global() to using mfn_t

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 07.07.15 at 12:07, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 07/07/15 11:01, Jan Beulich wrote:
>>>>> On 02.07.15 at 14:04, <Ben.Catterall@xxxxxxxxxx> wrote:
>>> --- a/xen/include/xen/domain_page.h
>>> +++ b/xen/include/xen/domain_page.h
>>> @@ -41,11 +41,15 @@ unsigned long domain_page_map_to_mfn(const void *va);
>>>   * address spaces (not just within the VCPU that created the mapping). 
> Global
>>>   * mappings can also be unmapped from any context.
>>>   */
>>> -void *map_domain_page_global(unsigned long mfn);
>>> +void *map_domain_page_global(mfn_t mfn);
>>>  void unmap_domain_page_global(const void *va);
>>>  
>>>  #define __map_domain_page(pg)        map_domain_page(__page_to_mfn(pg))
>>> -#define __map_domain_page_global(pg) 
> map_domain_page_global(__page_to_mfn(pg))
>>> +
>>> +static inline void *__map_domain_page_global(struct page_info *pg)
>> const
>>
>>> @@ -117,9 +121,17 @@ domain_mmap_cache_destroy(struct domain_mmap_cache 
> *cache)
>>>                                                        mfn_to_virt(smfn))
>>>  #define domain_page_map_to_mfn(va)          virt_to_mfn((unsigned 
> long)(va))
>>>  
>>> -#define map_domain_page_global(mfn)         mfn_to_virt(mfn)
>>> -#define __map_domain_page_global(pg)        page_to_virt(pg)
>>> -#define unmap_domain_page_global(va)        ((void)(va))
>>> +static inline void *map_domain_page_global(mfn_t mfn)
>>> +{
>>> +    return mfn_to_virt(mfn_x(mfn));
>>> +}
>>> +
>>> +static inline void *__map_domain_page_global(struct page_info *pg)
>> const
>>
>>> +{
>>> +    return page_to_virt(pg);
>>> +}
>>> +
>>> +static inline void unmap_domain_page_global(void *va) {};
>> And again (the more that the real function already has it that way).
> 
> Hmm.  Both unmap_domain_page() and _global() should be updated to not
> take a const void *va.
> 
> Just like free(), these functions are not performing a read-only
> operation on the destination pointer, therefore must not be performed on
> an actual const pointer.

I disagree - from the caller's persepctive they don't modify the data
being pointed to (that data is simply becoming undefined). An entity
allocating memory, initializing it, and never modifying it again should
be allowed to store the pointer in a variable pointing to a const
modified type, and free/unmap it without needing any casts. I.e. in
fact xfree() and free_xenheap_pages() should have their parameters
constified.

Jan


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