Re: [Xen-devel] [PATCH v5 17/17] xen/arm: p2m: Rework the interface of apply_p2m_changes and use typesafe

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 06/07/16 12:56, Julien Grall wrote:
> Hi Stefano,
>
> On 06/07/16 12:06, Stefano Stabellini wrote:
>> On Tue, 28 Jun 2016, Julien Grall wrote:
>>> @@ -1146,12 +1148,14 @@ out:
>>>       if ( rc < 0 && ( op == INSERT ) &&
>>>            addr != start_gpaddr )
>>>       {
>>> +        unsigned long gfn = paddr_to_pfn(addr);
>>> +
>>>           BUG_ON(addr == end_gpaddr);
>>>           /*
>>>            * addr keeps the address of the end of the last
>>> successfully-inserted
>>>            * mapping.
>>>            */
>>> -        apply_p2m_changes(d, REMOVE, start_gpaddr, addr, orig_maddr,
>>> +        apply_p2m_changes(d, REMOVE, sgfn, gfn - gfn_x(sgfn), smfn,
>>
>> Worth considering a gfn_sub (we already have gfn_add)?
>
> I thought about it. However the prototype of gfn_sub and gfn_add would
> be different.
>
> The former is subtracting two gfns whilst the latter is add a value to
> a gfn. So I think it would be confusing for the user. Although, I am
> open to any other suggestion.

This is the one problem with these boxed types; we have to reimplement
all the base operations ourselves.

I would caution against introducing too many helpers.  The manual unbox
and manipulate does work, and it should only be the very common
variations which should be abstracted.

~Andrew

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