[PATCH v2 0/3] xen/pvh: prevent Dom0 from accessing reserved MMIO regions

[PATCH v2 0/3] xen/pvh: prevent Dom0 from accessing reserved MMIO regions

[Roger Pau Monne]

This series aims to prevent PVH Dom0 from accessing MMIO regions of 
devices that are used by Xen.

Thanks for the review, Roger.

[PATCH v2 1/3] xen: allow set_mmio_p2m_entry to specify access type

[Roger Pau Monne]

Preparatory change that allows setting the access type to
set_mmio_p2m_entry.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Jun Nakajima <jun.nakajima@intel.com>
Cc: Eddie Dong <eddie.dong@intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Cc: Tim Deegan <tim@xen.org>
---
 xen/arch/x86/domain_build.c |  3 ++-
 xen/arch/x86/hvm/vmx/vmx.c  |  2 +-
 xen/arch/x86/mm/p2m.c       | 15 +++++++++------
 xen/include/asm-x86/p2m.h   |  3 ++-
 4 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/xen/arch/x86/domain_build.c b/xen/arch/x86/domain_build.c
index 7a912e9..f687c78 100644
--- a/xen/arch/x86/domain_build.c
+++ b/xen/arch/x86/domain_build.c
@@ -323,7 +323,8 @@ static __init void pvh_add_mem_mapping(struct domain *d, unsigned long gfn,
 
     for ( i = 0; i < nr_mfns; i++ )
     {
-        if ( (rc = set_mmio_p2m_entry(d, gfn + i, _mfn(mfn + i))) )
+        if ( (rc = set_mmio_p2m_entry(d, gfn + i, _mfn(mfn + i),
+                                      p2m_get_hostp2m(d)->default_access)) )
             panic("pvh_add_mem_mapping: gfn:%lx mfn:%lx i:%ld rc:%d\n",
                   gfn, mfn, i, rc);
         if ( !(i & 0xfffff) )
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 522892f..ab4b1e5 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2173,7 +2173,7 @@ static int vmx_alloc_vlapic_mapping(struct domain *d)
     share_xen_page_with_guest(virt_to_page(apic_va), d, XENSHARE_writable);
     d->arch.hvm_domain.vmx.apic_access_mfn = virt_to_mfn(apic_va);
     set_mmio_p2m_entry(d, paddr_to_pfn(APIC_DEFAULT_PHYS_BASE),
-        _mfn(virt_to_mfn(apic_va)));
+        _mfn(virt_to_mfn(apic_va)), p2m_get_hostp2m(d)->default_access);
 
     return 0;
 }
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index efa49dd..c1b7545 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -810,7 +810,7 @@ void p2m_change_type_range(struct domain *d,
 
 /* Returns: 0 for success, -errno for failure */
 static int set_typed_p2m_entry(struct domain *d, unsigned long gfn, mfn_t mfn,
-                               p2m_type_t gfn_p2mt)
+                               p2m_type_t gfn_p2mt, p2m_access_t access)
 {
     int rc = 0;
     p2m_access_t a;
@@ -837,7 +837,7 @@ static int set_typed_p2m_entry(struct domain *d, unsigned long gfn, mfn_t mfn,
 
     P2M_DEBUG("set %d %lx %lx\n", gfn_p2mt, gfn, mfn_x(mfn));
     rc = p2m_set_entry(p2m, gfn, mfn, PAGE_ORDER_4K, gfn_p2mt,
-                       p2m->default_access);
+                       access);
     gfn_unlock(p2m, gfn, 0);
     if ( rc )
         gdprintk(XENLOG_ERR,
@@ -850,12 +850,14 @@ static int set_typed_p2m_entry(struct domain *d, unsigned long gfn, mfn_t mfn,
 static int set_foreign_p2m_entry(struct domain *d, unsigned long gfn,
                                  mfn_t mfn)
 {
-    return set_typed_p2m_entry(d, gfn, mfn, p2m_map_foreign);
+    return set_typed_p2m_entry(d, gfn, mfn, p2m_map_foreign,
+                               p2m_get_hostp2m(d)->default_access);
 }
 
-int set_mmio_p2m_entry(struct domain *d, unsigned long gfn, mfn_t mfn)
+int set_mmio_p2m_entry(struct domain *d, unsigned long gfn, mfn_t mfn,
+                       p2m_access_t access)
 {
-    return set_typed_p2m_entry(d, gfn, mfn, p2m_mmio_direct);
+    return set_typed_p2m_entry(d, gfn, mfn, p2m_mmio_direct, access);
 }
 
 /* Returns: 0 for success, -errno for failure */
@@ -1858,7 +1860,8 @@ int map_mmio_regions(struct domain *d,
 
     for ( i = 0; !ret && i < nr; i++ )
     {
-        ret = set_mmio_p2m_entry(d, start_gfn + i, _mfn(mfn + i));
+        ret = set_mmio_p2m_entry(d, start_gfn + i, _mfn(mfn + i),
+                                 p2m_get_hostp2m(d)->default_access);
         if ( ret )
         {
             unmap_mmio_regions(d, start_gfn, i, mfn);
diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
index 2cf73ca..e86e26f 100644
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -513,7 +513,8 @@ int p2m_is_logdirty_range(struct p2m_domain *, unsigned long start,
                           unsigned long end);
 
 /* Set mmio addresses in the p2m table (for pass-through) */
-int set_mmio_p2m_entry(struct domain *d, unsigned long gfn, mfn_t mfn);
+int set_mmio_p2m_entry(struct domain *d, unsigned long gfn, mfn_t mfn,
+                       p2m_access_t access);
 int clear_mmio_p2m_entry(struct domain *d, unsigned long gfn, mfn_t mfn);
 
 /* Add foreign mapping to the guest's p2m table. */
-- 
1.9.3 (Apple Git-50)


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

Re: [PATCH v2 1/3] xen: allow set_mmio_p2m_entry to specify access type

[Jan Beulich]

>>> On 20.01.15 at 18:05, <roger.pau@citrix.com> wrote:
> --- a/xen/arch/x86/domain_build.c
> +++ b/xen/arch/x86/domain_build.c
> @@ -323,7 +323,8 @@ static __init void pvh_add_mem_mapping(struct domain *d, 
> unsigned long gfn,
>  
>      for ( i = 0; i < nr_mfns; i++ )
>      {
> -        if ( (rc = set_mmio_p2m_entry(d, gfn + i, _mfn(mfn + i))) )
> +        if ( (rc = set_mmio_p2m_entry(d, gfn + i, _mfn(mfn + i),
> +                                      p2m_get_hostp2m(d)->default_access)) )

Please latch this value prior to entering the loop.

Jan

[PATCH v2 2/3] xen/pvh: check permissions when adding MMIO regions

[Roger Pau Monne]

Check that MMIO regions added to PVH Dom0 are allowed. Previously a PVH Dom0
would have access to the full MMIO range.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
Changes since v1:
 - Use the newly introduced p2m_access_t to set the access type.
 - Don't add a next label.
---
 xen/arch/x86/domain_build.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/domain_build.c b/xen/arch/x86/domain_build.c
index f687c78..41d2541 100644
--- a/xen/arch/x86/domain_build.c
+++ b/xen/arch/x86/domain_build.c
@@ -319,12 +319,25 @@ static __init void pvh_add_mem_mapping(struct domain *d, unsigned long gfn,
                                        unsigned long mfn, unsigned long nr_mfns)
 {
     unsigned long i;
+    mfn_t omfn;
+    p2m_type_t t;
+    p2m_access_t a;
     int rc;
 
     for ( i = 0; i < nr_mfns; i++ )
     {
-        if ( (rc = set_mmio_p2m_entry(d, gfn + i, _mfn(mfn + i),
-                                      p2m_get_hostp2m(d)->default_access)) )
+        if ( !iomem_access_permitted(d, mfn + i, mfn + i) ) {
+            omfn = get_gfn_query_unlocked(d, gfn + i, &t);
+            guest_physmap_remove_page(d, gfn + i, mfn_x(omfn), PAGE_ORDER_4K);
+            continue;
+        }
+
+        if ( rangeset_contains_singleton(mmio_ro_ranges, mfn + i) )
+            a = p2m_access_r;
+        else
+            a = p2m_access_rw;
+
+        if ( (rc = set_mmio_p2m_entry(d, gfn + i, _mfn(mfn + i), a)) )
             panic("pvh_add_mem_mapping: gfn:%lx mfn:%lx i:%ld rc:%d\n",
                   gfn, mfn, i, rc);
         if ( !(i & 0xfffff) )
-- 
1.9.3 (Apple Git-50)


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

Re: [PATCH v2 2/3] xen/pvh: check permissions when adding MMIO regions

[Andrew Cooper]

On 20/01/15 17:05, Roger Pau Monne wrote:
> Check that MMIO regions added to PVH Dom0 are allowed. Previously a PVH Dom0
> would have access to the full MMIO range.
>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> Changes since v1:
>  - Use the newly introduced p2m_access_t to set the access type.
>  - Don't add a next label.
> ---
>  xen/arch/x86/domain_build.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/x86/domain_build.c b/xen/arch/x86/domain_build.c
> index f687c78..41d2541 100644
> --- a/xen/arch/x86/domain_build.c
> +++ b/xen/arch/x86/domain_build.c
> @@ -319,12 +319,25 @@ static __init void pvh_add_mem_mapping(struct domain *d, unsigned long gfn,
>                                         unsigned long mfn, unsigned long nr_mfns)
>  {
>      unsigned long i;
> +    mfn_t omfn;
> +    p2m_type_t t;
> +    p2m_access_t a;
>      int rc;
>  
>      for ( i = 0; i < nr_mfns; i++ )
>      {
> -        if ( (rc = set_mmio_p2m_entry(d, gfn + i, _mfn(mfn + i),
> -                                      p2m_get_hostp2m(d)->default_access)) )
> +        if ( !iomem_access_permitted(d, mfn + i, mfn + i) ) {
> +            omfn = get_gfn_query_unlocked(d, gfn + i, &t);
> +            guest_physmap_remove_page(d, gfn + i, mfn_x(omfn), PAGE_ORDER_4K);
> +            continue;
> +        }

This suggests a design flaw (possibly pre-existing).  We should not be
removing physmap entries in pvh_add_mem_mapping(), nor should we be a
position to need to revoke physmap entries during domain build.

If there is anything needing revoking at this stage, it should not have
been added earlier.  How did you come to introduce this code?

~Andrew

> +
> +        if ( rangeset_contains_singleton(mmio_ro_ranges, mfn + i) )
> +            a = p2m_access_r;
> +        else
> +            a = p2m_access_rw;
> +
> +        if ( (rc = set_mmio_p2m_entry(d, gfn + i, _mfn(mfn + i), a)) )
>              panic("pvh_add_mem_mapping: gfn:%lx mfn:%lx i:%ld rc:%d\n",
>                    gfn, mfn, i, rc);
>          if ( !(i & 0xfffff) )



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

Re: [PATCH v2 2/3] xen/pvh: check permissions when adding MMIO regions

[Roger Pau Monné]

El 20/01/15 a les 19.19, Andrew Cooper ha escrit:
> On 20/01/15 17:05, Roger Pau Monne wrote:
>> Check that MMIO regions added to PVH Dom0 are allowed. Previously a PVH Dom0
>> would have access to the full MMIO range.
>>
>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>> Cc: Jan Beulich <jbeulich@suse.com>
>> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
>> ---
>> Changes since v1:
>>  - Use the newly introduced p2m_access_t to set the access type.
>>  - Don't add a next label.
>> ---
>>  xen/arch/x86/domain_build.c | 17 +++++++++++++++--
>>  1 file changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/xen/arch/x86/domain_build.c b/xen/arch/x86/domain_build.c
>> index f687c78..41d2541 100644
>> --- a/xen/arch/x86/domain_build.c
>> +++ b/xen/arch/x86/domain_build.c
>> @@ -319,12 +319,25 @@ static __init void pvh_add_mem_mapping(struct domain *d, unsigned long gfn,
>>                                         unsigned long mfn, unsigned long nr_mfns)
>>  {
>>      unsigned long i;
>> +    mfn_t omfn;
>> +    p2m_type_t t;
>> +    p2m_access_t a;
>>      int rc;
>>  
>>      for ( i = 0; i < nr_mfns; i++ )
>>      {
>> -        if ( (rc = set_mmio_p2m_entry(d, gfn + i, _mfn(mfn + i),
>> -                                      p2m_get_hostp2m(d)->default_access)) )
>> +        if ( !iomem_access_permitted(d, mfn + i, mfn + i) ) {
>> +            omfn = get_gfn_query_unlocked(d, gfn + i, &t);
>> +            guest_physmap_remove_page(d, gfn + i, mfn_x(omfn), PAGE_ORDER_4K);
>> +            continue;
>> +        }
> 
> This suggests a design flaw (possibly pre-existing).  We should not be
> removing physmap entries in pvh_add_mem_mapping(), nor should we be a
> position to need to revoke physmap entries during domain build.
> 
> If there is anything needing revoking at this stage, it should not have
> been added earlier.  How did you come to introduce this code?

This code was introduced with the PVH Dom0 support series done by
Mukesh. Basically we let construct_dom0 build the physmap as it would be
done for a PV Dom0 (no holes at all, plain physmap from 0 to maxmem) and
then we punch the MMIO holes as needed. After punching the holes, we add
the leftover memory to the end of the memory map.

IMHO this seems better than having two different ways of building the
Dom0 memory map interleaved in the code, one for PV and one for PVH,
specially taking into account that the code in construct_dom0 is already
quite convoluted.

Roger.

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

Re: [PATCH v2 2/3] xen/pvh: check permissions when adding MMIO regions

[Andrew Cooper]

On 21/01/15 10:08, Roger Pau Monné wrote:
> El 20/01/15 a les 19.19, Andrew Cooper ha escrit:
>> On 20/01/15 17:05, Roger Pau Monne wrote:
>>> Check that MMIO regions added to PVH Dom0 are allowed. Previously a PVH Dom0
>>> would have access to the full MMIO range.
>>>
>>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>>> Cc: Jan Beulich <jbeulich@suse.com>
>>> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
>>> ---
>>> Changes since v1:
>>>  - Use the newly introduced p2m_access_t to set the access type.
>>>  - Don't add a next label.
>>> ---
>>>  xen/arch/x86/domain_build.c | 17 +++++++++++++++--
>>>  1 file changed, 15 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/xen/arch/x86/domain_build.c b/xen/arch/x86/domain_build.c
>>> index f687c78..41d2541 100644
>>> --- a/xen/arch/x86/domain_build.c
>>> +++ b/xen/arch/x86/domain_build.c
>>> @@ -319,12 +319,25 @@ static __init void pvh_add_mem_mapping(struct domain *d, unsigned long gfn,
>>>                                         unsigned long mfn, unsigned long nr_mfns)
>>>  {
>>>      unsigned long i;
>>> +    mfn_t omfn;
>>> +    p2m_type_t t;
>>> +    p2m_access_t a;
>>>      int rc;
>>>  
>>>      for ( i = 0; i < nr_mfns; i++ )
>>>      {
>>> -        if ( (rc = set_mmio_p2m_entry(d, gfn + i, _mfn(mfn + i),
>>> -                                      p2m_get_hostp2m(d)->default_access)) )
>>> +        if ( !iomem_access_permitted(d, mfn + i, mfn + i) ) {
>>> +            omfn = get_gfn_query_unlocked(d, gfn + i, &t);
>>> +            guest_physmap_remove_page(d, gfn + i, mfn_x(omfn), PAGE_ORDER_4K);
>>> +            continue;
>>> +        }
>> This suggests a design flaw (possibly pre-existing).  We should not be
>> removing physmap entries in pvh_add_mem_mapping(), nor should we be a
>> position to need to revoke physmap entries during domain build.
>>
>> If there is anything needing revoking at this stage, it should not have
>> been added earlier.  How did you come to introduce this code?
> This code was introduced with the PVH Dom0 support series done by
> Mukesh. Basically we let construct_dom0 build the physmap as it would be
> done for a PV Dom0 (no holes at all, plain physmap from 0 to maxmem) and
> then we punch the MMIO holes as needed. After punching the holes, we add
> the leftover memory to the end of the memory map.
>
> IMHO this seems better than having two different ways of building the
> Dom0 memory map interleaved in the code, one for PV and one for PVH,
> specially taking into account that the code in construct_dom0 is already
> quite convoluted.

PV domains do not have physmaps; physmaps are an HVM construct. 
Attempting to pretend that PV and PVH are the same when it comes to
memory setup like this is a misdesign at best.

Looking through the code, the memory setup for PVH appears completely
backwards, and should be fixed properly rather than having yet another
hack placed on top.

By (a very rushed) look of the code, it currently does
* Construct PV pagetables
* Construct plain p2m from 0 to dom0 max ram
* Wander over PV pagetables translating mfns to pfns
* Extra misc p2m changes including making holes, shifting ram, changing
permissions

A more sensible setup would be:
* Start constructing the p2m with the identity and and read-only areas
* Insert regular pages from 0 to max, skipping over existing areas
* Extract the kernel and initrd into p2m (the domain builder can
certainly do this)
* Set up identity pagetables covering the kernel and initrd

This way, there is no going back to undo something which was done
incorrectly earlier during build.  I presume I have over simplified some
areas, but I hope I have managed to get my point across.

~Andrew


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

Re: [PATCH v2 2/3] xen/pvh: check permissions when adding MMIO regions

[Tim Deegan]

At 18:19 +0000 on 20 Jan (1421774373), Andrew Cooper wrote:
> On 20/01/15 17:05, Roger Pau Monne wrote:
> > Check that MMIO regions added to PVH Dom0 are allowed. Previously a PVH Dom0
> > would have access to the full MMIO range.
> >
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > Cc: Jan Beulich <jbeulich@suse.com>
> > Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> > ---
> > Changes since v1:
> >  - Use the newly introduced p2m_access_t to set the access type.
> >  - Don't add a next label.
> > ---
> >  xen/arch/x86/domain_build.c | 17 +++++++++++++++--
> >  1 file changed, 15 insertions(+), 2 deletions(-)
> >
> > diff --git a/xen/arch/x86/domain_build.c b/xen/arch/x86/domain_build.c
> > index f687c78..41d2541 100644
> > --- a/xen/arch/x86/domain_build.c
> > +++ b/xen/arch/x86/domain_build.c
> > @@ -319,12 +319,25 @@ static __init void pvh_add_mem_mapping(struct domain *d, unsigned long gfn,
> >                                         unsigned long mfn, unsigned long nr_mfns)
> >  {
> >      unsigned long i;
> > +    mfn_t omfn;
> > +    p2m_type_t t;
> > +    p2m_access_t a;
> >      int rc;
> >  
> >      for ( i = 0; i < nr_mfns; i++ )
> >      {
> > -        if ( (rc = set_mmio_p2m_entry(d, gfn + i, _mfn(mfn + i),
> > -                                      p2m_get_hostp2m(d)->default_access)) )
> > +        if ( !iomem_access_permitted(d, mfn + i, mfn + i) ) {
> > +            omfn = get_gfn_query_unlocked(d, gfn + i, &t);
> > +            guest_physmap_remove_page(d, gfn + i, mfn_x(omfn), PAGE_ORDER_4K);
> > +            continue;
> > +        }
> 
> This suggests a design flaw (possibly pre-existing).  We should not be
> removing physmap entries in pvh_add_mem_mapping(), nor should we be a
> position to need to revoke physmap entries during domain build.
> 
> If there is anything needing revoking at this stage, it should not have
> been added earlier.

+1.  ISTR saying something like this before. 

Tim.

Re: [PATCH v2 2/3] xen/pvh: check permissions when adding MMIO regions

[Jan Beulich]

>>> On 20.01.15 at 18:05, <roger.pau@citrix.com> wrote:
> --- a/xen/arch/x86/domain_build.c
> +++ b/xen/arch/x86/domain_build.c
> @@ -319,12 +319,25 @@ static __init void pvh_add_mem_mapping(struct domain *d, unsigned long gfn,
>                                         unsigned long mfn, unsigned long nr_mfns)
>  {
>      unsigned long i;
> +    mfn_t omfn;
> +    p2m_type_t t;
> +    p2m_access_t a;
>      int rc;
>  
>      for ( i = 0; i < nr_mfns; i++ )
>      {
> -        if ( (rc = set_mmio_p2m_entry(d, gfn + i, _mfn(mfn + i),
> -                                      p2m_get_hostp2m(d)->default_access)) )
> +        if ( !iomem_access_permitted(d, mfn + i, mfn + i) ) {

Coding style.

Jan

[PATCH v2 3/3] xen: prevent access to HPET from Dom0

[Roger Pau Monne]

Prevent Dom0 from accessing HPET MMIO region by adding the HPET mfn to the
list of read-only memory regions.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
Changes since v1:
 - Instead of completely blocking access to the HPET mfn, set it as
   read-only.
---
 xen/arch/x86/domain_build.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/xen/arch/x86/domain_build.c b/xen/arch/x86/domain_build.c
index 41d2541..4c62bb8 100644
--- a/xen/arch/x86/domain_build.c
+++ b/xen/arch/x86/domain_build.c
@@ -36,6 +36,7 @@
 #include <asm/bzimage.h> /* for bzimage_parse */
 #include <asm/io_apic.h>
 #include <asm/hap.h>
+#include <asm/hpet.h> /* for hpet_address */
 
 #include <public/version.h>
 
@@ -1494,6 +1495,13 @@ int __init construct_dom0(
             rc |= iomem_deny_access(d, sfn, efn);
     }
 
+    /* Prevent access to HPET */
+    if ( hpet_address != 0 )
+    {
+        mfn = paddr_to_pfn(hpet_address);
+        rc |= rangeset_add_singleton(mmio_ro_ranges, mfn);
+    }
+
     BUG_ON(rc != 0);
 
     if ( elf_check_broken(&elf) )
-- 
1.9.3 (Apple Git-50)


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

Re: [PATCH v2 3/3] xen: prevent access to HPET from Dom0

[Andrew Cooper]

On 20/01/15 17:05, Roger Pau Monne wrote:
> Prevent Dom0 from accessing HPET MMIO region by adding the HPET mfn to the
> list of read-only memory regions.
>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>

Please introduce a command line parameter to control this, but
defaulting to read-only is perfectly fine.

I can well imagine that there are some bits of firmware around which
reuse the other 3/4 of the HPET page, and users with such hardware will
need a workaround.  Perhaps a boolean parameter named "ro_hpet" ?

~Andrew


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

Re: [PATCH v2 3/3] xen: prevent access to HPET from Dom0

[Roger Pau Monné]

El 20/01/15 a les 19.35, Andrew Cooper ha escrit:
> On 20/01/15 17:05, Roger Pau Monne wrote:
>> Prevent Dom0 from accessing HPET MMIO region by adding the HPET mfn to the
>> list of read-only memory regions.
>>
>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>> Cc: Jan Beulich <jbeulich@suse.com>
>> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> Please introduce a command line parameter to control this, but
> defaulting to read-only is perfectly fine.
> 
> I can well imagine that there are some bits of firmware around which
> reuse the other 3/4 of the HPET page, and users with such hardware will
> need a workaround.  Perhaps a boolean parameter named "ro_hpet" ?

Sure, sorry for that because I think it was mentioned in the last
version and I've completely forgot about it.

Roger.


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

Re: [PATCH v2 3/3] xen: prevent access to HPET from Dom0

[Jan Beulich]

>>> On 20.01.15 at 19:35, <andrew.cooper3@citrix.com> wrote:
> On 20/01/15 17:05, Roger Pau Monne wrote:
>> Prevent Dom0 from accessing HPET MMIO region by adding the HPET mfn to the
>> list of read-only memory regions.
>>
>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>> Cc: Jan Beulich <jbeulich@suse.com>
>> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> Please introduce a command line parameter to control this, but
> defaulting to read-only is perfectly fine.
> 
> I can well imagine that there are some bits of firmware around which
> reuse the other 3/4 of the HPET page, and users with such hardware will
> need a workaround.  Perhaps a boolean parameter named "ro_hpet" ?

For the "no-" prefix to look reasonable with it, "ro-hpet" please.

Jan

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

Re: [PATCH v2 3/3] xen: prevent access to HPET from Dom0

[Jan Beulich]

>>> On 20.01.15 at 18:05, <roger.pau@citrix.com> wrote:
> Prevent Dom0 from accessing HPET MMIO region by adding the HPET mfn to the
> list of read-only memory regions.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> Changes since v1:
>  - Instead of completely blocking access to the HPET mfn, set it as
>    read-only.

I'd actually prefer if you kept the ACPI based complete blocking, and
use the r/o variant only if ACPI didn't allow you to block access entirely.

Jan

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