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

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

[Roger Pau Monne]

This is needed so Xen can properly trap 4 byte accesses to 0xcf8 in order to
keep consistency with accesses to 0xcfc.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
 xen/arch/x86/setup.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 3b9aee5..c6e50cb 100644
--- 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;
 
     ASSERT(is_hardware_domain(d));
     if ( has_hvm_container_domain(d) )
@@ -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);
     }
 }
 
-- 
1.9.5 (Apple Git-50.3)


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

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

[Jan Beulich]

>>> On 08.05.15 at 12:00, <roger.pau@citrix.com> 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.

> @@ -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.

Jan

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

[Roger Pau Monné]

El 08/05/15 a les 12.18, Jan Beulich ha escrit:
>>>> On 08.05.15 at 12:00, <roger.pau@citrix.com> 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.

>> @@ -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.

Roger.

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

[Jan Beulich]

>>> On 08.05.15 at 12:22, <roger.pau@citrix.com> wrote:
> El 08/05/15 a les 12.18, Jan Beulich ha escrit:
>>>>> On 08.05.15 at 12:00, <roger.pau@citrix.com> 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