Re: [PATCH v3 2/2] ioreq-server: write protected range and forwarding

["Jan Beulich" <JBeulich@suse.com>]

>>> On 10.09.14 at 07:32, <wei.ye@intel.com> wrote:

> 
>> -----Original Message-----
>> From: xen-devel-bounces@lists.xen.org [mailto:xen-devel-
>> bounces@lists.xen.org] On Behalf Of Jan Beulich
>> Sent: Wednesday, September 3, 2014 9:17 PM
>> To: Ye, Wei
>> Cc: Tian, Kevin; keir@xen.org; ian.campbell@citrix.com;
>> stefano.stabellini@eu.citrix.com; tim@xen.org; ian.jackson@eu.citrix.com;
>> Dugger, Donald D; xen-devel@lists.xen.org; Paul.Durrant@citrix.com; Lv,
>> Zhiyuan; Zhang, Yang Z
>> Subject: Re: [Xen-devel] [PATCH v3 2/2] ioreq-server: write protected range
>> and forwarding
>> 
>> >>> On 03.09.14 at 23:53, <wei.ye@intel.com> wrote:
>> > +static int hvm_change_p2m_type_ioreq_server(struct domain *d,
>> uint16_t set,
>> > +                                            uint64_t start, uint64_t
>> > +end) {
>> > +    int rc = -EINVAL;
>> > +    uint64_t gpfn_s, gpfn_e, gpfn;
>> > +    p2m_type_t ot, nt;
>> > +
>> > +    if ( set )
>> > +    {
>> > +        ot = p2m_ram_rw;
>> > +        nt = p2m_mmio_write_dm;
>> > +    }
>> > +    else
>> > +    {
>> > +        ot = p2m_mmio_write_dm;
>> > +        nt = p2m_ram_rw;
>> > +    }
>> > +
>> > +    gpfn_s = start >> PAGE_SHIFT;
>> > +    gpfn_e = end >> PAGE_SHIFT;
>> 
>> Considering that the first really ought to be PFN_DOWN() - is the latter 
> really
>> correct? I'd rather expect that to be PFN_UP()...
>> 
> I think the latter one is also should be PFN_DOWN. Considering a range is 
> [0x0000, 0x1000],
> then only the first pfn 0 should be changed. Note that the follow loop:
> for (gpfn = gpfn_s; gpfn <= gpfn_e; gpfn++)
> The gpfn_e is included. If PFN_UP is to calculate the gpfn_e, there's wrong 
> p2m type change
> to pfn 1.

But then (i.e. if the range is inclusive) it still needs to be
PFN_UP(end - 1).

Jan