Re: [Xen-devel] [PATCH] xen/pvh: trap access to IO port range 0xcf8-0xcfb

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 08.05.15 at 12:22, <roger.pau@xxxxxxxxxx> wrote:
> El 08/05/15 a les 12.18, Jan Beulich ha escrit:
>>>>> On 08.05.15 at 12:00, <roger.pau@xxxxxxxxxx> wrote:
>>> --- a/xen/arch/x86/setup.c
>>> +++ b/xen/arch/x86/setup.c
>>> @@ -1527,7 +1527,7 @@ static int __hwdom_init io_bitmap_cb(unsigned long s, 
>>> unsigned long e,
>>>  
>>>  void __hwdom_init setup_io_bitmap(struct domain *d)
>>>  {
>>> -    int rc;
>>> +    int rc, i;
>> 
>> The new variable ought to be of unsigned type and could go
>> into the more narrow scope it's needed in.
> 
> Why of type unsigned? __set_bit expects an int.

This doesn't matter as long as the value fits in both an int and an
unsigned int. (I also - sadly only now - realized that your other
patch used plain int in a similar case where it should have been
unsigned, along with a more complex ASSERT() expression than
really necessary.)

>>> @@ -1535,6 +1535,12 @@ void __hwdom_init setup_io_bitmap(struct domain *d)
>>>          rc = rangeset_report_ranges(d->arch.ioport_caps, 0, 0x10000,
>>>                                      io_bitmap_cb, d);
>>>          BUG_ON(rc);
>>> +        /*
>>> +         * NB: we need to trap accesses to the range 0xcf8-0xcfb in order
>>> +         * to intercept 4 byte accesses.
>>> +         */
>>> +        for ( i = 0xcf8; i < 0xcfc; i++ )
>>> +            __set_bit(i, d->arch.hvm_domain.io_bitmap);
>> 
>> To achieve what the comment says you'd really only need to
>> set any one of the four bits.
> 
> Indeed, but I think it's more clear this way, the effect is exactly the
> same. If you prefer I can only set 0xcf8 and we can get rid of the extra
> i variable.

I think that would be the preferable solution.

Jan


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