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

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



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

> 
>> -----Original Message-----
>> From: xen-devel-bounces@xxxxxxxxxxxxx [mailto:xen-devel-
>> bounces@xxxxxxxxxxxxx] On Behalf Of Jan Beulich
>> Sent: Wednesday, September 3, 2014 9:17 PM
>> To: Ye, Wei
>> Cc: Tian, Kevin; keir@xxxxxxx; ian.campbell@xxxxxxxxxx;
>> stefano.stabellini@xxxxxxxxxxxxx; tim@xxxxxxx; ian.jackson@xxxxxxxxxxxxx;
>> Dugger, Donald D; xen-devel@xxxxxxxxxxxxx; Paul.Durrant@xxxxxxxxxx; 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@xxxxxxxxx> 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


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


 


Rackspace

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