Re: [Xen-devel] [PATCH 3/6] xentrace: P2M lookup suport for ARM platform

2016-03-19 Thread Julien Grall

Hello Benjamin,

Thank you for the patch.

On 16/03/16 20:51, Benjamin Sanda wrote:

From: bensanda 

Modified p2m_lookup() to provide support for xentrace on the ARM platform. 
Added check for DOMID_XEN which skips PFN to MFN translation. xentrace sends a 
MFN dirrectly when requesting DOMID_XEN, so no translation is needed. Also sets 
page memory type, p2m_type_t, to p2m_ram_rw to provide correct access.


A line in the commit message should not be longer than 72 characters.



Signed-off-by: Benjamin Sanda 
---
  xen/arch/arm/p2m.c | 19 +++
  1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index a2a9c4b..2e7da43 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -228,10 +228,21 @@ paddr_t p2m_lookup(struct domain *d, paddr_t paddr, 
p2m_type_t *t)
  paddr_t ret;
  struct p2m_domain *p2m = &d->arch.p2m;

-spin_lock(&p2m->lock);
-ret = __p2m_lookup(d, paddr, t);
-spin_unlock(&p2m->lock);
-
+/* Check for DOMID_XEN: If we are called with DOMID_XEN (from xentrace)


Multi-lines comment in Xen should be:

/*
 * Foo
 * Bar
 */

You can find the coding style in CODING_STYLE.


+then paddr is already a MFN and no translation is needed. We only set the
+page type as p2m_raw_rw and return the MFN directly */


DOMID_XEN is not specific to xentrace. It's a mechanism to share xenheap 
page to any guest.


xentrace is using directly an MFN because DOMID_XEN is considered as a 
PV guest on x86 (i.e MFN == GFN). And we don't have a such concept on ARM.



+if(DOMID_XEN != d->domain_id)



if ( d->domain_id != DOMID_XEN )


+{
+spin_lock(&p2m->lock);
+ret = __p2m_lookup(d, paddr, t);
+spin_unlock(&p2m->lock);
+}
+else
+{
+*t = p2m_ram_rw;


A DOMID_XEN page could be read only too. For instance, the meta-data of 
the trace buffer is read-only (see t_info), we don't want a domain to be 
able to overwrite them.


However, all the foreign page are mapped read-write. You will need to 
rework the code to map a foreign domain (see XENMAPSPACE_gmfn_foreign) 
to allow read-only foreign page (maybe by adding a new p2m_type_t?).



+ret = paddr;
+    }
+
  return ret;
  }




Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH 4/6] xentrace: ARM platform DOMID_XEN mapping support

2016-03-20 Thread Julien Grall

Hello Benjamin,

Thank you for the patch.

On 16/03/16 20:51, Benjamin Sanda wrote:

From: bensanda 

Modified xenmem_add_to_physmap_one() to provide support for xentrace on the ARM 
platform. Checks for DOMID_XEN added via new function, get_pg_owner, ported 
from x86 code base. This provides correct calls to rcu_lock_domain() when 
DOMID_XEN is requested. DOMID_XEN checks also adde to skip page to MFN 
translation (xentrace sends a MFN dirrectly and so does not need to be 
translated).


A line in the commit message should not be longer than 72 characters.



Signed-off-by: Benjamin Sanda 
---
  xen/arch/arm/mm.c | 56 +--
  1 file changed, 54 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 81f9e2e..b1d834f 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -41,6 +41,7 @@
  #include 

  struct domain *dom_xen, *dom_io, *dom_cow;
+static struct domain *get_pg_owner(domid_t domid);


I would rather avoid to forward declare a function if there is no strict 
dependency on other functions. Instead, I would add the function before 
the caller.




  /* Static start-of-day pagetables that we use before the allocators
   * are up. These are used by all CPUs during bringup before switching
@@ -1099,7 +1100,8 @@ int xenmem_add_to_physmap_one(
  {
  struct domain *od;
  p2m_type_t p2mt;
-od = rcu_lock_domain_by_any_id(foreign_domid);
+od = get_pg_owner(foreign_domid);
+
  if ( od == NULL )
  return -ESRCH;

@@ -1132,7 +1134,17 @@ int xenmem_add_to_physmap_one(
  return -EINVAL;
  }

-mfn = page_to_mfn(page);
+/* If DOMID_XEN then no page to MFN translation is
+needed as we already have the MFN directly */
+if(DOMID_XEN !=od->domain_id)
+{
+mfn = page_to_mfn(page);
+}
+else
+{
+mfn = idx;
+}
+


Please avoid to spread the DOMID_ID specific case everywhere. The cost 
to calculate the MFN of a page is very limited.



  t = p2m_map_foreign;

  rcu_unlock_domain(od);
@@ -1312,6 +1324,46 @@ void clear_and_clean_page(struct page_info *page)
  unmap_domain_page(p);
  }

+/* Ported from x86 architecture: checks for special domain requests for
+DOMID_XEN or DOMID_IO which must be handled differently then guest domain
+requests */
+static struct domain *get_pg_owner(domid_t domid)


This function is very similar to the x86 one. I think it would benefit 
to implement get_pg_owner in the common code and add arch specific 
helper when it's necessary.


Also, please introduce the helper put_pg_owner to stay consistent.


+{
+struct domain *pg_owner = NULL, *curr = current->domain;
+
+if ( likely(domid == DOMID_SELF) )
+{
+pg_owner = rcu_lock_current_domain();
+goto out;
+}
+
+if ( unlikely(domid == curr->domain_id) )
+{
+goto out;
+}
+
+/* check for special domain cases of DOMID_IO or DOMID_XEN which
+must use rcu_lock_domain() and dom_xen/dom_io as the domid_t */
+switch ( domid )
+{
+case DOMID_IO:
+pg_owner = rcu_lock_domain(dom_io);
+break;
+case DOMID_XEN:
+pg_owner = rcu_lock_domain(dom_xen);
+break;
+default:
+if ( (pg_owner = rcu_lock_domain_by_id(domid)) == NULL )
+{
+break;
+}
+break;
+}
+
+ out:
+return pg_owner;
+}
+
  /*
   * Local variables:
   * mode: C



Regards,

--
Julien Grall

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


Re: [Xen-devel] List of projects for 4.7

2016-03-21 Thread Julien Grall

(Strip the CC list)

On 19/03/2016 05:45, Shannon Zhao wrote:

On 2016年03月18日 20:07, Wei Liu wrote:

Hi all

Today is that last posting day for new features. And we are two weeks
away from the anticipated freeze date.

I've gone through the outstanding patch series on the list and ask for
input from various core community members. I've enumerated a list
here, which covers several areas of this release and can be used as a
guideline.

Important and close patch, new features:
1. xSplice
2. CPUID levelling
3. ARM ACPI


For ARM ACPI, there are about 7 patches which don't get reviewed-by or
acked-by. It still needs some efforts and maybe another two versions to
make them in good shape I think.


I will review your patch series today.

Cheers,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v6 02/22] arm/acpi: Add a helper function to get the acpi table offset

2016-03-21 Thread Julien Grall

Hi Shannon,

On 17/03/2016 09:40, Shannon Zhao wrote:

From: Shannon Zhao 

These tables are aligned with 64bit.

Signed-off-by: Shannon Zhao 
---
  xen/arch/arm/acpi/lib.c| 15 +++
  xen/include/asm-arm/acpi.h |  2 ++
  2 files changed, 17 insertions(+)

diff --git a/xen/arch/arm/acpi/lib.c b/xen/arch/arm/acpi/lib.c
index db5c4d8..79f7edd 100644
--- a/xen/arch/arm/acpi/lib.c
+++ b/xen/arch/arm/acpi/lib.c
@@ -60,3 +60,18 @@ bool_t __init acpi_psci_hvc_present(void)
  {
  return acpi_gbl_FADT.arm_boot_flags & ACPI_FADT_PSCI_USE_HVC;
  }
+
+paddr_t __init acpi_get_table_offset(struct membank tbl_add[],
+ EFI_MEM_RES index)


Without looking at the callers, the usage of this function is very 
unclear. For instance what does mean the offset in this case?


Furthermore, you are assuming that all the tables before the given index 
have all been created and will never be modified. So all the code flow 
to generate the ACPI tables is inflexible.


I'm fine for now with this kind of assumption, but this need to be spell 
out in the header.



+{
+int i;
+paddr_t offset = 0;
+
+for ( i = 0; i < index; i++ )
+{
+/* Aligned with 64bit (8 bytes) */
+offset += ROUNDUP(tbl_add[i].size, 8);
+}
+
+return offset;
+}
diff --git a/xen/include/asm-arm/acpi.h b/xen/include/asm-arm/acpi.h
index 7f59761..569fc31 100644
--- a/xen/include/asm-arm/acpi.h
+++ b/xen/include/asm-arm/acpi.h
@@ -25,6 +25,7 @@

  #include 
  #include 
+#include 

  #define COMPILER_DEPENDENT_INT64   long long
  #define COMPILER_DEPENDENT_UINT64  unsigned long long
@@ -45,6 +46,7 @@ typedef enum {
  bool_t __init acpi_psci_present(void);
  bool_t __init acpi_psci_hvc_present(void);
  void __init acpi_smp_init_cpus(void);
+paddr_t acpi_get_table_offset(struct membank tbl_add[], EFI_MEM_RES index);

  #ifdef CONFIG_ACPI
  extern bool_t acpi_disabled;



Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v6 03/22] arm/acpi: Prepare FADT table for Dom0

2016-03-21 Thread Julien Grall

Hi Shannon,

On 17/03/2016 09:40, Shannon Zhao wrote:

From: Shannon Zhao 

Copy and modify FADT table before passing it to Dom0. Set PSCI_COMPLIANT
and PSCI_USE_HVC.

Signed-off-by: Shannon Zhao 
Reviewed-by: Stefano Stabellini 


Acked-by: Julien Grall 

Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v6 04/22] arm/gic: Add a new callback for creating MADT table for Dom0

2016-03-21 Thread Julien Grall

Hi Shannon,

On 17/03/2016 09:40, Shannon Zhao wrote:

From: Shannon Zhao 

Add a new member in gic_hw_operations which is used to creat MADT table


s/create/create/


for Dom0.

Signed-off-by: Shannon Zhao 
Reviewed-by: Stefano Stabellini 
---
  xen/arch/arm/gic-v2.c | 34 ++
  xen/arch/arm/gic-v3.c | 47 +++
  xen/arch/arm/gic.c|  5 +
  xen/include/asm-arm/gic.h |  3 +++
  4 files changed, 89 insertions(+)

diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index 0fcb894..02db5f2 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -685,6 +685,35 @@ static void __init gicv2_dt_init(void)
  }

  #ifdef CONFIG_ACPI
+static u32 gicv2_make_hwdom_madt(const struct domain *d, u32 offset)
+{
+struct acpi_subtable_header *header;
+struct acpi_madt_generic_interrupt *host_gicc, *gicc;
+u32 i, size, table_len = 0;
+u8 *base_ptr = d->arch.efi_acpi_table + offset;
+
+header = acpi_table_get_entry_madt(ACPI_MADT_TYPE_GENERIC_INTERRUPT, 0);
+if ( !header )
+panic("Can't get GICC entry");


I would prefer if you return an error here. In the future we may want to 
support hardware domain creation later (see the xen option hardware_dom).



+host_gicc = container_of(header, struct acpi_madt_generic_interrupt,
+ header);
+
+size = sizeof(struct acpi_madt_generic_interrupt);
+/* Add Generic Interrupt */
+for ( i = 0; i < d->max_vcpus; i++ )
+{
+gicc = (struct acpi_madt_generic_interrupt *)(base_ptr + table_len);
+ACPI_MEMCPY(gicc, host_gicc, size);
+gicc->cpu_interface_number = i;
+gicc->uid = i;
+gicc->flags = ACPI_MADT_ENABLED;
+gicc->arm_mpidr = vcpuid_to_vaffinity(i);


What about the other fields?

* parking_version: DOM0 will always use PSCI => should be set to 0
* performance_interrupt: There is currently PMU support for DOM0
* gic{v,h}_base_address/vgic_interrupt: they are used by Xen => should 
not be exposed to the guest



+table_len += size;
+}
+
+return table_len;
+}
+
  static int __init
  gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header,
  const unsigned long end)


[...]


diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index f83fd88..d9fce4b 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1236,6 +1236,48 @@ static void __init gicv3_dt_init(void)
  }

  #ifdef CONFIG_ACPI
+static u32 gicv3_make_hwdom_madt(const struct domain *d, u32 offset)
+{
+struct acpi_subtable_header *header;
+struct acpi_madt_generic_interrupt *host_gicc, *gicc;
+struct acpi_madt_generic_redistributor *gicr;
+u8 *base_ptr = d->arch.efi_acpi_table + offset;
+u32 i, table_len = 0, size;
+
+/* Add Generic Interrupt */
+header = acpi_table_get_entry_madt(ACPI_MADT_TYPE_GENERIC_INTERRUPT, 0);
+if ( !header )
+panic("Can't get GICC entry");
+host_gicc = container_of(header, struct acpi_madt_generic_interrupt,
+ header);
+
+size = sizeof(struct acpi_madt_generic_interrupt);
+for ( i = 0; i < d->max_vcpus; i++ )
+{
+gicc = (struct acpi_madt_generic_interrupt *)(base_ptr + table_len);
+ACPI_MEMCPY(gicc, host_gicc, size);
+gicc->cpu_interface_number = i;
+gicc->uid = i;
+gicc->flags = ACPI_MADT_ENABLED;
+gicc->arm_mpidr = vcpuid_to_vaffinity(i);
+table_len += size;


Ditto for the fields.

Furthermore, you use the Generic Redistributor table to describe the 
redistributor regions. So the field gicr_base_address should be set to 0.



+}
+
+/* Add Generic Redistributor */
+size = sizeof(struct acpi_madt_generic_redistributor);
+for ( i = 0; i < d->arch.vgic.nr_regions; i++ )
+{
+gicr = (struct acpi_madt_generic_redistributor *)(base_ptr + 
table_len);
+gicr->header.type = ACPI_MADT_TYPE_GENERIC_REDISTRIBUTOR;
+gicr->header.length = size;
+gicr->base_address = d->arch.vgic.rdist_regions[i].base;
+gicr->length = d->arch.vgic.rdist_regions[i].size;
+table_len += size;
+}
+
+return table_len;
+}
+
  static int __init
  gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header,
  const unsigned long end)


[...]

Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v6 05/22] arm/acpi: Prepare MADT table for Dom0

2016-03-21 Thread Julien Grall

Hi Shannon,

On 17/03/2016 09:40, Shannon Zhao wrote:

From: Shannon Zhao 

Copy main MADT table contents and distributor subtable from physical
ACPI MADT table. Make other subtables through the callback of
gic_hw_ops.

Signed-off-by: Shannon Zhao 
Reviewed-by: Stefano Stabellini 


With the 2 changes mentioned below:

Acked-by: Julien Grall 


---
  xen/arch/arm/domain_build.c | 50 +
  1 file changed, 50 insertions(+)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index d9b7213..ed257e0 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1357,6 +1357,52 @@ static int prepare_dtb(struct domain *d, struct 
kernel_info *kinfo)
  }

  #ifdef CONFIG_ACPI
+static int acpi_create_madt(struct domain *d, struct membank tbl_add[])
+{
+struct acpi_table_header *table = NULL;
+struct acpi_table_madt *madt = NULL;
+struct acpi_subtable_header *header;
+struct acpi_madt_generic_distributor *gicd;
+u32 table_size = sizeof(struct acpi_table_madt);
+u32 offset = acpi_get_table_offset(tbl_add, TBL_MADT);
+acpi_status status;
+u8 *base_ptr, checksum;
+
+status = acpi_get_table(ACPI_SIG_MADT, 0, &table);
+
+if ( ACPI_FAILURE(status) )
+{
+const char *msg = acpi_format_exception(status);
+
+printk("Failed to get MADT table, %s\n", msg);
+return -EINVAL;
+}
+
+base_ptr = d->arch.efi_acpi_table + offset;
+ACPI_MEMCPY(base_ptr, table, table_size);
+
+/* Add Generic Distributor */
+header = acpi_table_get_entry_madt(ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR, 0);
+if ( !header )
+panic("Can't get GICD entry");


Please avoid panic when it's possible to return an error.


+gicd = container_of(header, struct acpi_madt_generic_distributor, header);
+ACPI_MEMCPY(base_ptr + table_size, gicd,
+sizeof(struct acpi_madt_generic_distributor));
+table_size += sizeof(struct acpi_madt_generic_distributor);
+
+/* Add other subtables*/


NIT: Missing space before */


+table_size += gic_make_hwdom_madt(d, offset + table_size);
+
+madt = (struct acpi_table_madt *)base_ptr;
+madt->header.length = table_size;
+checksum = acpi_tb_checksum(ACPI_CAST_PTR(u8, madt), table_size);
+madt->header.checksum -= checksum;
+
+tbl_add[TBL_MADT].start = d->arch.efi_acpi_gpa + offset;
+tbl_add[TBL_MADT].size = table_size;
+
+return 0;
+}

  static int acpi_create_fadt(struct domain *d, struct membank tbl_add[])
  {


[...]

Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v6 06/22] arm/acpi: Prepare STAO table for Dom0

2016-03-21 Thread Julien Grall

Hi Shannon,

On 17/03/2016 09:40, Shannon Zhao wrote:

From: Shannon Zhao 

Create STAO table for Dom0. This table is used to tell Dom0 whether it
should ignore UART defined in SPCR table or the ACPI namespace names.

Look at below url for details:
http://wiki.xenproject.org/mediawiki/images/0/02/Status-override-table.pdf

Signed-off-by: Parth Dixit 
Signed-off-by: Shannon Zhao 
Reviewed-by: Stefano Stabellini 


With the change mentioned below:

Acked-by: Julien Grall 


---
  xen/arch/arm/domain_build.c | 41 +
  1 file changed, 41 insertions(+)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index ed257e0..b369f2e 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1357,6 +1357,43 @@ static int prepare_dtb(struct domain *d, struct 
kernel_info *kinfo)
  }

  #ifdef CONFIG_ACPI
+static int acpi_create_stao(struct domain *d, struct membank tbl_add[])
+{
+struct acpi_table_header *table = NULL;
+struct acpi_table_stao *stao = NULL;
+u32 table_size = sizeof(struct acpi_table_stao);
+u32 offset = acpi_get_table_offset(tbl_add, TBL_STAO);
+acpi_status status;
+u8 *base_ptr, checksum;
+
+/* Copy OEM and ASL compiler fields from another table, use MADT */
+status = acpi_get_table(ACPI_SIG_MADT, 0, &table);
+
+if ( ACPI_FAILURE(status) )
+{
+const char *msg = acpi_format_exception(status);
+
+printk("Failed to get MADT table, %s\n", msg);


NIT: You use the same error message in 2 different place. Can you add a 
prefix to help finding the error quicker?



+return -EINVAL;
+}
+
+base_ptr = d->arch.efi_acpi_table + offset;
+ACPI_MEMCPY(base_ptr, table, sizeof(struct acpi_table_header));
+
+stao = (struct acpi_table_stao *)base_ptr;
+ACPI_MEMCPY(stao->header.signature, ACPI_SIG_STAO, 4);
+stao->header.revision = 1;
+stao->header.length = table_size;
+stao->ignore_uart = 1;
+checksum = acpi_tb_checksum(ACPI_CAST_PTR(u8, stao), table_size);
+stao->header.checksum -= checksum;
+
+tbl_add[TBL_STAO].start = d->arch.efi_acpi_gpa + offset;
+tbl_add[TBL_STAO].size = table_size;
+
+return 0;
+}
+
  static int acpi_create_madt(struct domain *d, struct membank tbl_add[])
  {
  struct acpi_table_header *table = NULL;


Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v6 07/22] arm/acpi: Prepare XSDT table for Dom0

2016-03-21 Thread Julien Grall

Hi Shannon,

On 17/03/2016 09:40, Shannon Zhao wrote:

From: Shannon Zhao 

Copy and modify XSDT table before passing it to Dom0. Repalce the entry


s/Repalce/Replace/


value of the copied table. Add a new entry for STAO table as well. And
keep entry value of other reused tables unchanged.

Signed-off-by: Shannon Zhao 
Acked-by: Stefano Stabellini 


Acked-by: Julien Grall 

Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v6 09/22] arm/p2m: Add helper functions to map memory regions

2016-03-21 Thread Julien Grall

Title: to map/unmap

On 17/03/2016 09:40, Shannon Zhao wrote:

diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 433952a..17be6ad 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -144,6 +144,16 @@ int p2m_cache_flush(struct domain *d, xen_pfn_t start_mfn, 
xen_pfn_t end_mfn);
  /* Setup p2m RAM mapping for domain d from start-end. */
  int p2m_populate_ram(struct domain *d, paddr_t start, paddr_t end);

+int map_regions_rw(struct domain *d,
+unsigned long start_gfn,
+unsigned long nr_mfns,
+unsigned long mfn);


From the commit message, this function will map the region read-write 
and cacheable.


But it's not clear from the name. Please either rename the function or 
document it in the code.



+
+int unmap_regions_rw(struct domain *d,
+unsigned long start_gfn,
+unsigned long nr_mfns,
+unsigned long mfn);
+
  int guest_physmap_add_entry(struct domain *d,
  unsigned long gfn,
  unsigned long mfn,



Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v6 10/22] arm/acpi: Map all other tables for Dom0

2016-03-21 Thread Julien Grall

Title: Map all other tables into DOM0 memory.

On 17/03/2016 09:40, Shannon Zhao wrote:

From: Shannon Zhao 

Map all other tables to Dom0 using 1:1 mappings.


s/to/into/



Signed-off-by: Shannon Zhao 
Reviewed-by: Stefano Stabellini 
---
  xen/arch/arm/domain_build.c | 26 ++
  1 file changed, 26 insertions(+)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index ea7d6a5..c71976c 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1357,6 +1357,30 @@ static int prepare_dtb(struct domain *d, struct 
kernel_info *kinfo)
  }

  #ifdef CONFIG_ACPI
+static void acpi_map_other_tables(struct domain *d)
+{
+int i;
+unsigned long res;
+u64 addr, size;
+
+/* Map all other tables to Dom0 using 1:1 mappings. */


That's not really true. AFAICT, you will map all the ACPI tables, 
included the ones that have been discarded.



+for( i = 0; i < acpi_gbl_root_table_list.count; i++ )
+{
+addr = acpi_gbl_root_table_list.tables[i].address;
+size = acpi_gbl_root_table_list.tables[i].length;
+res = map_regions_rw(d,
+ paddr_to_pfn(addr & PAGE_MASK),
+ DIV_ROUND_UP(size, PAGE_SIZE),
+ paddr_to_pfn(addr & PAGE_MASK));
+if ( res )
+{
+ panic(XENLOG_ERR "Unable to map 0x%"PRIx64
+   " - 0x%"PRIx64" in domain \n",
+   addr & PAGE_MASK, PAGE_ALIGN(addr + size) - 1);


"Unable to map ACPI region ..." to differentiate with the error message 
in map_range_to_domain.



+}
+}
+}
+
  static int acpi_create_rsdp(struct domain *d, struct membank tbl_add[])
  {

@@ -1661,6 +1685,8 @@ static int prepare_acpi(struct domain *d, struct 
kernel_info *kinfo)
  if ( rc != 0 )
  return rc;

+acpi_map_other_tables(d);
+
  return 0;
  }
  #else



Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v6 08/22] arm/acpi: Prepare RSDP table for Dom0

2016-03-21 Thread Julien Grall

Hi Shannon,

On 17/03/2016 09:40, Shannon Zhao wrote:

From: Shannon Zhao 

Copy RSDP table and replace rsdp->xsdt_physical_address with new address

  ^ the

of XSDT table, so it can point to the right XSDT table.

Signed-off-by: Shannon Zhao 
Acked-by: Stefano Stabellini 


With the 2 changes mentioned:

Acked-by: Julien Grall 


---
  xen/arch/arm/domain_build.c | 36 
  1 file changed, 36 insertions(+)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index f9fe289..ea7d6a5 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1357,6 +1357,38 @@ static int prepare_dtb(struct domain *d, struct 
kernel_info *kinfo)
  }

  #ifdef CONFIG_ACPI
+static int acpi_create_rsdp(struct domain *d, struct membank tbl_add[])
+{
+
+struct acpi_table_rsdp *rsdp = NULL;
+u64 addr;
+u64 table_size = sizeof(struct acpi_table_rsdp);
+u8 *base_ptr;
+u8 checksum;
+
+addr = acpi_os_get_root_pointer();
+if( !addr )
+panic("Unable to get acpi root pointer\n");


Please avoid panic when it's possible.


+
+rsdp = acpi_os_map_memory(addr, table_size);
+base_ptr = d->arch.efi_acpi_table
+   + acpi_get_table_offset(tbl_add, TBL_RSDP);
+ACPI_MEMCPY(base_ptr, rsdp, table_size);
+acpi_os_unmap_memory(rsdp, table_size);
+
+rsdp = (struct acpi_table_rsdp *)base_ptr;
+/* Replace xsdt_physical_address */
+rsdp->xsdt_physical_address = tbl_add[TBL_XSDT].start;
+checksum = acpi_tb_checksum(ACPI_CAST_PTR(u8, rsdp), table_size);
+rsdp->checksum = rsdp->checksum - checksum;
+
+tbl_add[TBL_RSDP].start = d->arch.efi_acpi_gpa
+  + acpi_get_table_offset(tbl_add, TBL_RSDP);
+tbl_add[TBL_RSDP].size = table_size;
+
+return 0;
+}
+
  static void acpi_xsdt_modify_entry(u64 entry[], unsigned long entry_count,
 char *signature, u64 addr)
  {
@@ -1625,6 +1657,10 @@ static int prepare_acpi(struct domain *d, struct 
kernel_info *kinfo)
  if ( rc != 0 )
  return rc;

+rc = acpi_create_rsdp(d, tbl_add);
+if ( rc != 0 )
+return rc;
+
  return 0;
  }
  #else



Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v6 11/22] arm/acpi: Prepare EFI system table for Dom0

2016-03-21 Thread Julien Grall

Hi Shannon,

On 17/03/2016 09:40, Shannon Zhao wrote:

[...]


diff --git a/xen/arch/arm/efi/efi-dom0.c b/xen/arch/arm/efi/efi-dom0.c
index 90a7699..b8a062c 100644
--- a/xen/arch/arm/efi/efi-dom0.c
+++ b/xen/arch/arm/efi/efi-dom0.c
@@ -48,3 +48,47 @@ size_t __init estimate_efi_size(int mem_nr_banks)

  return size;
  }
+
+#include "../../../common/decompress.h"
+#define XZ_EXTERN STATIC
+#include "../../../common/xz/crc32.c"


All the includes should be at the beginning of the file.


+
+void __init acpi_create_efi_system_table(paddr_t paddr, void *efi_acpi_table,
+ struct membank tbl_add[])
+{
+u64 table_addr, table_size, offset = 0;
+u8 *base_ptr;
+EFI_CONFIGURATION_TABLE *efi_conf_tbl;
+EFI_SYSTEM_TABLE *efi_sys_tbl;
+
+table_addr = paddr + acpi_get_table_offset(tbl_add, TBL_EFIT);
+table_size = sizeof(EFI_SYSTEM_TABLE) + sizeof(EFI_CONFIGURATION_TABLE)
+ + sizeof(xen_efi_fw_vendor);
+base_ptr = efi_acpi_table + acpi_get_table_offset(tbl_add, TBL_EFIT);
+efi_sys_tbl = (EFI_SYSTEM_TABLE *)base_ptr;
+
+efi_sys_tbl->Hdr.Signature = EFI_SYSTEM_TABLE_SIGNATURE;
+/* Specify the revision as 2.5 */
+efi_sys_tbl->Hdr.Revision = (2 << 16 | 50);
+efi_sys_tbl->Hdr.HeaderSize = table_size;
+
+efi_sys_tbl->FirmwareRevision = 1;
+efi_sys_tbl->NumberOfTableEntries = 1;
+offset += sizeof(EFI_SYSTEM_TABLE);
+memcpy((CHAR16 *)(base_ptr + offset), xen_efi_fw_vendor,


Why the cast to CHAR16?


+   sizeof(xen_efi_fw_vendor));
+efi_sys_tbl->FirmwareVendor = (CHAR16 *)(table_addr + offset);
+
+offset += sizeof(xen_efi_fw_vendor);
+efi_conf_tbl = (EFI_CONFIGURATION_TABLE *)(base_ptr + offset);
+efi_conf_tbl->VendorGuid = (EFI_GUID)ACPI_20_TABLE_GUID;
+efi_conf_tbl->VendorTable = (VOID *)tbl_add[TBL_RSDP].start;
+efi_sys_tbl->ConfigurationTable = (EFI_CONFIGURATION_TABLE *)(table_addr
+  + offset);
+xz_crc32_init();
+efi_sys_tbl->Hdr.CRC32 = xz_crc32((uint8_t *)efi_sys_tbl,
+  efi_sys_tbl->Hdr.HeaderSize, 0);
+
+tbl_add[TBL_EFIT].start = table_addr;
+tbl_add[TBL_EFIT].size = table_size;
+}


[...]

Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v6 12/22] arm/acpi: Prepare EFI memory descriptor for Dom0

2016-03-21 Thread Julien Grall

Hi Shannon,

On 17/03/2016 09:40, Shannon Zhao wrote:

From: Shannon Zhao 

Create a few EFI memory descriptors to tell Dom0 the RAM region


s/a few//


information, ACPI table regions and EFI tables reserved resions.


s/resions/regions/



Signed-off-by: Parth Dixit 
Signed-off-by: Shannon Zhao 
---
v6: remove acpi_diabled check
---
  xen/arch/arm/domain_build.c |  2 ++
  xen/arch/arm/efi/efi-dom0.c | 40 
  xen/include/asm-arm/setup.h |  5 +
  3 files changed, 47 insertions(+)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 613551c..008fc76 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1688,6 +1688,8 @@ static int prepare_acpi(struct domain *d, struct 
kernel_info *kinfo)
  acpi_map_other_tables(d);
  acpi_create_efi_system_table(d->arch.efi_acpi_gpa, d->arch.efi_acpi_table,
   tbl_add);
+acpi_create_efi_mmap_table(d->arch.efi_acpi_gpa, d->arch.efi_acpi_len,
+   d->arch.efi_acpi_table, &kinfo->mem, tbl_add);

  return 0;
  }
diff --git a/xen/arch/arm/efi/efi-dom0.c b/xen/arch/arm/efi/efi-dom0.c
index b8a062c..3ffde94 100644
--- a/xen/arch/arm/efi/efi-dom0.c
+++ b/xen/arch/arm/efi/efi-dom0.c
@@ -23,6 +23,7 @@

  #include "efi.h"
  #include "efi-dom0.h"
+#include 
  #include 
  #include 

@@ -92,3 +93,42 @@ void __init acpi_create_efi_system_table(paddr_t paddr, void 
*efi_acpi_table,
  tbl_add[TBL_EFIT].start = table_addr;
  tbl_add[TBL_EFIT].size = table_size;
  }
+
+void __init acpi_create_efi_mmap_table(paddr_t paddr, paddr_t size,


Please rename paddr and size to a more meaningful name. Like: acpi_gpa 
and acpi_len.


Actually, you could directly pass the domain. So you will pass one 
argument rather 3 (paddr, size and efi_acpi_table).



+   void *efi_acpi_table,
+   const struct meminfo *mem,


Please pass kinfo instead.


+   struct membank tbl_add[])
+{
+EFI_MEMORY_DESCRIPTOR *memory_map;
+unsigned int i, offset;
+u8 *base_ptr;
+
+base_ptr = efi_acpi_table + acpi_get_table_offset(tbl_add, TBL_MMAP);
+memory_map = (EFI_MEMORY_DESCRIPTOR *)(base_ptr);


NIT: the bracket around base_ptr are not necessary.


+
+offset = 0;
+for( i = 0; i < mem->nr_banks; i++, offset++ )
+{
+memory_map[offset].Type = EfiConventionalMemory;
+memory_map[offset].PhysicalStart = mem->bank[i].start;
+memory_map[offset].NumberOfPages = PFN_UP(mem->bank[i].size);


The page size use by Xen and UEFI may be different. Please use 
EFI_SIZE_TO_PAGES here.



+memory_map[offset].Attribute = EFI_MEMORY_WB;
+}
+
+for( i = 0; i < acpi_mem.nr_banks; i++, offset++ )
+{
+memory_map[offset].Type = EfiACPIReclaimMemory;
+memory_map[offset].PhysicalStart = acpi_mem.bank[i].start;
+memory_map[offset].NumberOfPages = PFN_UP(acpi_mem.bank[i].size);


Ditto

You are also assuming that acpi_mem.bank[i].size will always be aligned 
to 4KB. If so, we may expose unwanted data to the guest.


Based on how the field is set, I would add a BUG_ON to ensure this 
condition.



+memory_map[offset].Attribute = EFI_MEMORY_WB;
+}
+
+memory_map[offset].Type = EfiACPIReclaimMemory;
+memory_map[offset].PhysicalStart = paddr;
+memory_map[offset].NumberOfPages = PFN_UP(size);
+memory_map[offset].Attribute = EFI_MEMORY_WB;
+
+tbl_add[TBL_MMAP].start = paddr + acpi_get_table_offset(tbl_add, TBL_MMAP);
+tbl_add[TBL_MMAP].size = sizeof(EFI_MEMORY_DESCRIPTOR)
+ * (mem->nr_banks + acpi_mem.nr_banks + 1);
+}
diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
index e423b15..b2899f2 100644
--- a/xen/include/asm-arm/setup.h
+++ b/xen/include/asm-arm/setup.h
@@ -56,6 +56,11 @@ size_t estimate_efi_size(int mem_nr_banks);
  void acpi_create_efi_system_table(paddr_t paddr, void *efi_acpi_table,
struct membank tbl_add[]);

+void acpi_create_efi_mmap_table(paddr_t paddr, paddr_t size,
+void *efi_acpi_table,
+const struct meminfo *mem,
+struct membank tbl_add[]);
+
  int construct_dom0(struct domain *d);

  void discard_initial_modules(void);



Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v6 01/22] arm/acpi: Estimate memory required for acpi/efi tables

2016-03-21 Thread Julien Grall

Hi Shannon,

On 17/03/2016 09:40, Shannon Zhao wrote:

[...]

+static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
+{
+int rc = 0;
+int order;
+
+rc = estimate_acpi_efi_size(d, kinfo);
+if ( rc != 0 )
+return rc;
+
+order = get_order_from_bytes(d->arch.efi_acpi_len);
+d->arch.efi_acpi_table = alloc_xenheap_pages(order, 0);
+if ( d->arch.efi_acpi_table == NULL )
+{
+printk("unable to allocate memory!\n");
+return -ENOMEM;
+}
+memset(d->arch.efi_acpi_table, 0, d->arch.efi_acpi_len);
+
+/* For ACPI, Dom0 doesn't use kinfo->gnttab_start to get the grant table
+ * region. So we use it as the ACPI table mapped address. */
+d->arch.efi_acpi_gpa = kinfo->gnttab_start;


I forgot to mention it on the last mail. If you re-use the gnttab 
region, you need to make sure that the region size will be enough to fit 
the ACPI tables.


It will help for debugging if we reach the limit in the future.


+
+    return 0;


[...]

Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v6 13/22] arm/acpi: Map the new created EFI and ACPI tables to Dom0

2016-03-21 Thread Julien Grall

Hi Shannon,

On 17/03/2016 09:40, Shannon Zhao wrote:

From: Shannon Zhao 

Map the UEFI and ACPI tables which we created to non-RAM space in Dom0.

Signed-off-by: Shannon Zhao 
Reviewed-by: Stefano Stabellini 
---
  xen/arch/arm/domain_build.c | 15 +++
  1 file changed, 15 insertions(+)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 008fc76..e036887 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1691,6 +1691,21 @@ static int prepare_acpi(struct domain *d, struct 
kernel_info *kinfo)
  acpi_create_efi_mmap_table(d->arch.efi_acpi_gpa, d->arch.efi_acpi_len,
 d->arch.efi_acpi_table, &kinfo->mem, tbl_add);

+/* Map the EFI and ACPI tables to Dom0 */
+rc = map_regions_rw(d,
+paddr_to_pfn(d->arch.efi_acpi_gpa),
+PFN_UP(d->arch.efi_acpi_len),
+paddr_to_pfn(virt_to_maddr(d->arch.efi_acpi_table)));


The ACPI/EFI tables could potentially have data in the cache but are not 
written into the memory (because Xen is mapping the RAM with caching 
enabled). However, DOM0 may decide to map it with cache disabled. 
Therefore it would be possible for the domain to see wrong data.


So I think you need to clean the cache for this region.


+if ( rc != 0 )
+{
+printk(XENLOG_ERR "Unable to map 0x%"PRIx64


"Unable to map EFI/ACPI table ..." to differentiate with a similar error 
message in map_range_to_domain



+   " - 0x%"PRIx64" in domain %d\n",
+   d->arch.efi_acpi_gpa & PAGE_MASK,
+   PAGE_ALIGN(d->arch.efi_acpi_gpa + d->arch.efi_acpi_len) - 1,
+   d->domain_id);
+    return rc;
+}
+
  return 0;
  }
  #else



Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v6 14/22] arm/acpi: Create min DT stub for Dom0

2016-03-21 Thread Julien Grall
chosen_node(const struct kernel_info *kinfo,


Please prefix it with acpi_ to make clear it's ACPI specific.


+struct membank tbl_add[])


Why do you pass tbl_add here? You don't use it within the function.


+{
+int res;
+const char *bootargs = NULL;
+const struct bootmodule *mod = kinfo->kernel_bootmodule;
+void *fdt = kinfo->fdt;
+
+DPRINT("Create chosen node\n");
+res = fdt_begin_node(fdt, "chosen");
+if ( res )
+return res;
+
+if ( mod && mod->cmdline[0] )
+{
+bootargs = &mod->cmdline[0];
+res = fdt_property(fdt, "bootargs", bootargs, strlen(bootargs) + 1);
+if ( res )
+   return res;
+}
+
+/*
+ * If the bootloader provides an initrd, we must create a placeholder
+ * for the initrd properties. The values will be replaced later.
+ */
+if ( mod && mod->size )
+{
+u64 a = 0;
+res = fdt_property(kinfo->fdt, "linux,initrd-start", &a, sizeof(a));
+if ( res )
+return res;
+
+res = fdt_property(kinfo->fdt, "linux,initrd-end", &a, sizeof(a));
+if ( res )
+return res;
+}
+
+res = fdt_end_node(fdt);
+
+return res;
+}


[...]


@@ -1706,6 +1845,10 @@ static int prepare_acpi(struct domain *d, struct 
kernel_info *kinfo)
  return rc;
  }

+rc = create_acpi_dtb(kinfo, tbl_add);
+if ( rc != 0 )
+return rc;
+
  return 0;
  }
  #else
diff --git a/xen/arch/arm/efi/efi-dom0.c b/xen/arch/arm/efi/efi-dom0.c
index 3ffde94..688fa8b 100644
--- a/xen/arch/arm/efi/efi-dom0.c
+++ b/xen/arch/arm/efi/efi-dom0.c
@@ -24,6 +24,7 @@
  #include "efi.h"
  #include "efi-dom0.h"
  #include 
+#include 
  #include 
  #include 

@@ -132,3 +133,50 @@ void __init acpi_create_efi_mmap_table(paddr_t paddr, 
paddr_t size,
  tbl_add[TBL_MMAP].size = sizeof(EFI_MEMORY_DESCRIPTOR)
   * (mem->nr_banks + acpi_mem.nr_banks + 1);
  }
+
+/* Create place holder for efi values. */


Placeholder means we will create an empty space and replace by proper 
values later.


However, you directly create the properties with proper values.


+int __init arm_acpi_make_efi_nodes(void *fdt, struct membank tbl_add[])


It's odd to have this function in efi-dom0.c. We want to keep all the 
device tree creation together.


Also, to stay consistent with the other name. Please rename the function 
into acpi_make_efi_nodes.



+{
+u64 fdt_val64;
+u32 fdt_val32;
+int desc_ver = 1;
+int res;
+
+res = fdt_begin_node(fdt, "uefi");
+if ( res )
+return res;
+
+fdt_val64 = cpu_to_fdt64(tbl_add[TBL_EFIT].start);
+res = fdt_property(fdt, "xen,uefi-system-table",
+   &fdt_val64, sizeof(fdt_val64));


Those two lines could be replaced by fdt_property_u64.


+if ( res )
+return res;
+
+fdt_val64 = cpu_to_fdt64(tbl_add[TBL_MMAP].start);
+res = fdt_property(fdt, "xen,uefi-mmap-start",
+   &fdt_val64,  sizeof(fdt_val64));


Ditto


+if ( res )
+return res;
+
+fdt_val32 = cpu_to_fdt32(tbl_add[TBL_MMAP].size);
+res = fdt_property(fdt, "xen,uefi-mmap-size",
+   &fdt_val32,  sizeof(fdt_val32));


Here by fdt_property_u32.


+if ( res )
+return res;
+
+fdt_val32 = cpu_to_fdt32(sizeof(EFI_MEMORY_DESCRIPTOR));
+res = fdt_property(fdt, "xen,uefi-mmap-desc-size",
+ &fdt_val32, sizeof(fdt_val32));


Ditto


+if ( res )
+return res;
+
+fdt_val32 = cpu_to_fdt32(desc_ver);
+res = fdt_property(fdt, "xen,uefi-mmap-desc-ver",
+ &fdt_val32, sizeof(fdt_val32));


Ditto


+if ( res )
+return res;
+
+res = fdt_end_node(fdt);
+
+return res;
+}
diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
index b2899f2..05f0210 100644
--- a/xen/include/asm-arm/setup.h
+++ b/xen/include/asm-arm/setup.h
@@ -61,6 +61,8 @@ void acpi_create_efi_mmap_table(paddr_t paddr, paddr_t size,
  const struct meminfo *mem,
  struct membank tbl_add[]);

+int arm_acpi_make_efi_nodes(void *fdt, struct membank tbl_add[]);
+
  int construct_dom0(struct domain *d);

  void discard_initial_modules(void);



Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v6 09/22] arm/p2m: Add helper functions to map memory regions

2016-03-22 Thread Julien Grall

Hi Shannon,

On 22/03/16 13:05, Shannon Zhao wrote:

On 2016年03月21日 23:52, Julien Grall wrote:

Title: to map/unmap

On 17/03/2016 09:40, Shannon Zhao wrote:

diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 433952a..17be6ad 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -144,6 +144,16 @@ int p2m_cache_flush(struct domain *d, xen_pfn_t
start_mfn, xen_pfn_t end_mfn);
   /* Setup p2m RAM mapping for domain d from start-end. */
   int p2m_populate_ram(struct domain *d, paddr_t start, paddr_t end);

+int map_regions_rw(struct domain *d,
+unsigned long start_gfn,
+unsigned long nr_mfns,
+unsigned long mfn);


 From the commit message, this function will map the region read-write
and cacheable.

But it's not clear from the name. Please either rename the function or
document it in the code.

So map_regions_rw_cache?


It sounds good.

Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v6 12/22] arm/acpi: Prepare EFI memory descriptor for Dom0

2016-03-22 Thread Julien Grall

Hi Shannon,

On 22/03/16 13:16, Shannon Zhao wrote:

On 2016年03月22日 00:51, Julien Grall wrote:

+memory_map[offset].Attribute = EFI_MEMORY_WB;
+}
+
+for( i = 0; i < acpi_mem.nr_banks; i++, offset++ )
+{
+memory_map[offset].Type = EfiACPIReclaimMemory;
+memory_map[offset].PhysicalStart = acpi_mem.bank[i].start;
+memory_map[offset].NumberOfPages =
PFN_UP(acpi_mem.bank[i].size);


Ditto

You are also assuming that acpi_mem.bank[i].size will always be aligned
to 4KB. If so, we may expose unwanted data to the guest.

Based on how the field is set, I would add a BUG_ON to ensure this
condition.

UEFI spec says
"EFI memory descriptors of type EfiACPIReclaimMemory and
EfiACPIMemoryNVS must be aligned on a 4 KiB boundary and must be a
multiple of 4 KiB in size."

So I think the size is aligned to 4kb, right?


Right. I was suggested to add a BUG_ON to document the constraint and 
ensure nobody will play with acpi_mem outside EFI.


A such check would also be nice for mem->bank[i].size;

Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v6 13/22] arm/acpi: Map the new created EFI and ACPI tables to Dom0

2016-03-22 Thread Julien Grall

Hi Shannon,

On 22/03/16 13:18, Shannon Zhao wrote:

On 2016年03月22日 08:42, Julien Grall wrote:

Hi Shannon,

On 17/03/2016 09:40, Shannon Zhao wrote:

From: Shannon Zhao 

Map the UEFI and ACPI tables which we created to non-RAM space in Dom0.

Signed-off-by: Shannon Zhao 
Reviewed-by: Stefano Stabellini 
---
   xen/arch/arm/domain_build.c | 15 +++
   1 file changed, 15 insertions(+)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 008fc76..e036887 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1691,6 +1691,21 @@ static int prepare_acpi(struct domain *d,
struct kernel_info *kinfo)
   acpi_create_efi_mmap_table(d->arch.efi_acpi_gpa,
d->arch.efi_acpi_len,
  d->arch.efi_acpi_table, &kinfo->mem,
tbl_add);

+/* Map the EFI and ACPI tables to Dom0 */
+rc = map_regions_rw(d,
+paddr_to_pfn(d->arch.efi_acpi_gpa),
+PFN_UP(d->arch.efi_acpi_len),
+
paddr_to_pfn(virt_to_maddr(d->arch.efi_acpi_table)));


The ACPI/EFI tables could potentially have data in the cache but are not
written into the memory (because Xen is mapping the RAM with caching
enabled). However, DOM0 may decide to map it with cache disabled.
Therefore it would be possible for the domain to see wrong data.

So I think you need to clean the cache for this region.

Oh, that would be good. Is there any existing function I can use?


You could reuse p2m_cache_flush. However this function will only flush 
cache for p2m_ram_* entries.


I think the best way would be to extend the CACHEFLUSH operations and 
maybe p2m_cache_flush (?).


Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v6 15/22] arm/acpi: Permit access all Xen unused SPIs for Dom0

2016-03-22 Thread Julien Grall

Hi Shannon,

On 17/03/16 09:41, Shannon Zhao wrote:

From: Shannon Zhao 

Permit access all Xen unused SPIs for Dom0 except the interrupts that
Xen uses.


You say exactly the same things with all "Xen unused SPIs for Dom0" and 
"except the interrupts that Xen uses".


I would instead say:

"Allow DOM0 to use all SPIs but the ones used by Xen."


Then when Dom0 configures the interrupt, it could set the
interrupt type and route it to Dom0.

Signed-off-by: Shannon Zhao 
Reviewed-by: Stefano Stabellini 
---
  xen/arch/arm/domain_build.c | 31 +++
  1 file changed, 31 insertions(+)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 6726e45..1e5ee0e 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1359,6 +1359,33 @@ static int prepare_dtb(struct domain *d, struct 
kernel_info *kinfo)
  #ifdef CONFIG_ACPI
  #define ACPI_DOM0_FDT_MIN_SIZE 4096

+static int acpi_permit_spi_access(struct domain *d)
+{
+int i, res;
+struct irq_desc *desc;
+
+/* Here just permit Dom0 to access the SPIs which Xen doesn't use. Then 
when


Coding style:

/*
 * FOo
 * Bar
 */


+ * Dom0 configures the interrupt, set the interrupt type and route it to
+ * Dom0.
+ */
+for( i = NR_LOCAL_IRQS; i < vgic_num_irqs(d); i++ )
+{
+desc = irq_to_desc(i);
+if( desc->action != NULL)


Well some of the SPIs used by Xen may not be registered yet. For 
instance the SMMU driver doesn't register any SPIs until it's necessary 
(i.e a device is assigned to a domain).



+continue;
+
+res = irq_permit_access(d, i);
+if ( res )
+{
+printk(XENLOG_ERR "Unable to permit to dom%u access to IRQ %u\n",
+   d->domain_id, i);
+return res;
+}
+}
+
+return 0;
+}
+
  static int make_chosen_node(const struct kernel_info *kinfo,
  struct membank tbl_add[])
  {
@@ -1849,6 +1876,10 @@ static int prepare_acpi(struct domain *d, struct 
kernel_info *kinfo)
  if ( rc != 0 )
  return rc;

+rc = acpi_permit_spi_access(d);
+if ( rc != 0 )
+    return rc;
+
  return 0;
  }
  #else



--
Julien Grall

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


Re: [Xen-devel] [PATCH v6 16/22] arm/acpi: Configure SPI interrupt type and route to Dom0 dynamically

2016-03-22 Thread Julien Grall

Hi Shannon,

On 17/03/16 09:41, Shannon Zhao wrote:

From: Shannon Zhao 

Interrupt information is described in DSDT and is not available at the
time of booting. Check if the interrupt is permitted to access and set
the interrupt type, route it to guest dynamically only for SPI
and Dom0.

Signed-off-by: Parth Dixit 
Signed-off-by: Shannon Zhao 
---
v6: coding style
---
  xen/arch/arm/vgic.c | 32 
  1 file changed, 32 insertions(+)

diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index ee35683..39d858c 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -25,6 +25,8 @@
  #include 
  #include 
  #include 
+#include 
+#include 

  #include 

@@ -334,6 +336,19 @@ void vgic_disable_irqs(struct vcpu *v, uint32_t r, int n)
  }
  }

+#define VGIC_ICFG_MASK(intr) (1 << ((2 * ((intr) % 16)) + 1))
+
+static inline unsigned int get_the_irq_type(struct vcpu *v, int n, int index)
+{
+struct vgic_irq_rank *vr = vgic_get_rank(v, n);
+uint32_t tr = vr->icfg[index >> 4];
+
+if ( tr & VGIC_ICFG_MASK(index) )
+return IRQ_TYPE_EDGE_BOTH;
+else
+return IRQ_TYPE_LEVEL_MASK;
+}
+
  void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
  {
  const unsigned long mask = r;
@@ -342,9 +357,26 @@ void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
  unsigned long flags;
  int i = 0;
  struct vcpu *v_target;
+struct domain *d = v->domain;
+int ret;

  while ( (i = find_next_bit(&mask, 32, i)) < 32 ) {
  irq = i + (32 * n);
+/* Set the irq type and route it to guest only for SPI and Dom0 */
+if( irq_access_permitted(d, irq) && is_hardware_domain(d) &&
+( irq >= 32 ) && ( !acpi_disabled ) )
+{
+ret = irq_set_spi_type(irq, get_the_irq_type(v, n, i));
+if ( ret )
+printk(XENLOG_WARNING "The irq type is not correct\n");


XENLOG_WARNING (and XENLOG_ERR) are not rate-limited. Therefore a domain 
(even if it's dom0) could flood the console.


Please use XENLOG_G_* instead. However in this case, "v" is the current 
vCPU so you can use gprintk(XENLOG_WARNING, ...);



+
+vgic_reserve_virq(d, irq);
+
+ret = route_irq_to_guest(d, irq, irq, NULL);
+if ( ret )
+printk(XENLOG_ERR "Unable to route IRQ %u to domain %u\n",
+   irq, d->domain_id);


I'm not against this solution for short term. But in long term we would 
benefit to split the IRQ configuration from the routing.


The routing will be done before DOM0 is booting. The IRQ configuration 
will just end up to write in the ICFGR register.


This will also help for PCI-passthrough as the guest will have to 
configure the SPIs (we can't expect DOM0 doing it for it). But the 
routing will be done ahead.



+}
  v_target = __vgic_get_target_vcpu(v, irq);
  p = irq_to_pending(v_target, irq);
  set_bit(GIC_IRQ_GUEST_ENABLED, &p->status);



Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v6 17/22] arm/gic: Add a new callback to deny Dom0 access to GIC regions

2016-03-22 Thread Julien Grall
0644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -360,6 +360,8 @@ struct gic_hw_operations {
const struct dt_device_node *gic, void *fdt);
  /* Create MADT table for the hardware domain */
  u32 (*make_hwdom_madt)(const struct domain *d, u32 offset);
+/* Deny access to GIC regions */
+int (*iomem_deny_access)(const struct domain *d);
  };

  void register_gic_ops(const struct gic_hw_operations *ops);
@@ -367,6 +369,7 @@ int gic_make_hwdom_dt_node(const struct domain *d,
 const struct dt_device_node *gic,
 void *fdt);
  u32 gic_make_hwdom_madt(const struct domain *d, u32 offset);
+int gic_iomem_deny_access(const struct domain *d);

  #endif /* __ASSEMBLY__ */
  #endif



Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v2] arm: Fix asynchronous aborts (SError exceptions) due to bogus PTEs

2016-03-22 Thread Julien Grall

(CC some ARM folks)

On 21/03/2016 23:18, Shanker Donthineni wrote:

Hi Julien,


Hello Shanker,

Sorry for the late answer.


Do you have any other comments to be addressed?


I have a question regarding the implication for what you wrote in the 
commit.


As far as I understand, any speculative table walk might cause an 
imprecise asynchronous abort. So if a guest is using page tables that 
contain garbage, it would be possible to receive an SError. Am I right?




On 03/16/2016 02:08 PM, Shanker Donthineni wrote:

From: Vikram Sethi 

ARMv8 architecture allows performing prefetch data/instructions
from memory locations marked as normal memory. Prefetch does not
mean that the data/instruction has to be used/executed in code
flow. All PTEs that appear to be valid to MMU must contain valid
physical address with proper attributes otherwise MMU table walk
might cause imprecise asynchronous aborts.

The way current XEN code is preparing page tables for frametable
and xenheap memory can create bogus PTEs. This patch fixes the
issue by clearing page table memory before populating EL2 L0/L1
PTEs. Without this patch XEN crashes on Qualcomm Technologies
server chips due to asynchronous aborts.

The speculative/prefetch feature explanation is scattered everywhere
in ARM specification but below two sections have useful information.

E2.8 Memory types and attributes
G4.12.6 External abort on a translation table walk


As said on an earlier version of this patch, please mention the version 
of the spec when you quote it.




Signed-off-by: Vikram Sethi 
Signed-off-by: Shanker Donthineni 
---
Changes since v1:
 Replace memset() with clear_page()
 Edit commit description

  xen/arch/arm/mm.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 81f9e2e..3fda8f3 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -730,6 +730,8 @@ void __init setup_xenheap_mappings(unsigned long base_mfn,
  else
  {
  unsigned long first_mfn = alloc_boot_pages(1, 1);
+
+clear_page(mfn_to_virt(first_mfn));
  pte = mfn_to_xen_entry(first_mfn, WRITEALLOC);
  pte.pt.table = 1;
  write_pte(p, pte);
@@ -773,6 +775,7 @@ void __init setup_frametable_mappings(paddr_t ps, paddr_t 
pe)
  second = mfn_to_virt(second_base);
  for ( i = 0; i < nr_second; i++ )
  {
+clear_page(mfn_to_virt(second_base + i));
  pte = mfn_to_xen_entry(second_base + i, WRITEALLOC);
  pte.pt.table = 1;
  write_pte(&xen_first[first_table_offset(FRAMETABLE_VIRT_START)+i], 
pte);




Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v4 07/34] arm/x86: Use struct virtual_region to do bug, symbol, and (x86) exception tables

2016-03-23 Thread Julien Grall

Hi Jan,

On 23/03/16 08:19, Jan Beulich wrote:

On 22.03.16 at 21:18,  wrote:

+symbols_lookup_t *symbols_lookup;
+
+struct {
+const struct bug_frame *bugs; /* The pointer to array of bug
frames. */
+ssize_t n_bugs; /* The number of them. */
+} frame[BUGFRAME_NR];
+
+#ifdef CONFIG_X86
+struct exception_table_entry *ex;
+struct exception_table_entry *ex_end;
+#endif


Would there be any harm omitting the #ifdef and leaving the
pointers be NULL on ARM?


The structure exception_table_entry is only defined for x86 
(asm-x86/uaccess.h).


Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v6 18/22] arm/acpi: Permit MMIO access of Xen unused devices for Dom0

2016-03-23 Thread Julien Grall

Hi Shannon,

On 17/03/16 09:41, Shannon Zhao wrote:

From: Shannon Zhao 

Firstly it permits full MMIO capabilities for Dom0. Then deny MMIO
access of Xen used devices, such as UART, GIC, SMMU. Currently, it only
denies the MMIO access of UART and GIC regions. For other Xen used
devices it could be added later when they are supported.

Signed-off-by: Shannon Zhao 
Reviewed-by: Stefano Stabellini 


Acked-by: Julien Grall 


---
  xen/arch/arm/domain_build.c | 36 
  1 file changed, 36 insertions(+)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 1e5ee0e..a4abf28 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1359,6 +1359,38 @@ static int prepare_dtb(struct domain *d, struct 
kernel_info *kinfo)
  #ifdef CONFIG_ACPI
  #define ACPI_DOM0_FDT_MIN_SIZE 4096

+static int acpi_iomem_deny_access(struct domain *d)
+{
+acpi_status status;
+struct acpi_table_spcr *spcr = NULL;
+unsigned long gfn;
+int rc;
+
+/* Firstly permit full MMIO capabilities. */
+rc = iomem_permit_access(d, 0UL, ~0UL);
+if ( rc )
+return rc;
+
+/* TODO: Deny MMIO access for SMMU, GIC ITS */
+status = acpi_get_table(ACPI_SIG_SPCR, 0,
+(struct acpi_table_header **)&spcr);
+
+if ( ACPI_FAILURE(status) )
+{
+printk("Failed to get SPCR table\n");
+return -EINVAL;
+}
+
+gfn = spcr->serial_port.address >> PAGE_SHIFT;
+/* Deny MMIO access for UART */
+rc = iomem_deny_access(d, gfn, gfn + 1);
+if ( rc )
+return rc;
+
+/* Deny MMIO access for GIC regions */
+return gic_iomem_deny_access(d);
+}
+
  static int acpi_permit_spi_access(struct domain *d)
  {
  int i, res;
@@ -1880,6 +1912,10 @@ static int prepare_acpi(struct domain *d, struct 
kernel_info *kinfo)
  if ( rc != 0 )
  return rc;

+rc = acpi_iomem_deny_access(d);
+if ( rc != 0 )
+return rc;
+
  return 0;
  }
  #else



Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v6 20/22] xen/acpi: Fix event-channel interrupt when booting with ACPI

2016-03-23 Thread Julien Grall

Hi Shannon,

On 17/03/16 09:41, Shannon Zhao wrote:

From: Shannon Zhao 

Store the event-channel interrupt number and flag in HVM parameter
HVM_PARAM_CALLBACK_IRQ. Then Dom0 could get it through hypercall
HVMOP_get_param.

Signed-off-by: Shannon Zhao 
Reviewed-by: Stefano Stabellini 
---
  xen/arch/arm/domain_build.c | 10 ++
  1 file changed, 10 insertions(+)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index a4abf28..dcbcc4c 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2008,6 +2008,7 @@ static void initrd_load(struct kernel_info *kinfo)
  static void evtchn_fixup(struct domain *d, struct kernel_info *kinfo)
  {
  int res, node;
+u64 val;
  gic_interrupt_t intr;

  /*
@@ -2023,6 +2024,15 @@ static void evtchn_fixup(struct domain *d, struct 
kernel_info *kinfo)
  printk("Allocating PPI %u for event channel interrupt\n",
 d->arch.evtchn_irq);

+/* Set the value of domain param HVM_PARAM_CALLBACK_IRQ */
+val = (u64)HVM_PARAM_CALLBACK_TYPE_PPI << 56;
+val |= (2 << 8); /* Active-low level-sensitive  */
+val |= d->arch.evtchn_irq & 0xff;
+d->arch.hvm_domain.params[HVM_PARAM_CALLBACK_IRQ] = val;
+
+if ( !acpi_disabled )
+return;


Please add a comment to explain that you can only get the event channel 
interrupt via hypercall when ACPI is used.



+
  /* Fix up "interrupts" in /hypervisor node */
  node = fdt_path_offset(kinfo->fdt, "/hypervisor");
  if ( node < 0 )



Regards,

--
Julien Grall

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


[Xen-devel] Building tools for ARM (WAS Re: help)

2016-03-23 Thread Julien Grall

(CC xen-user, BCC xen-devel)

On 23/03/16 10:23, Marwa Hamza wrote:

ello


Hello,

Please have a more meaningful subject. Also the question is not related 
to development so the thread is moved to xen-users.



i'm trying to learn more about xen hypervisor .. i install xen in my
host with alpine as domu
and now i'm trying to build xen from source with linux dom0 for an arm
board .. i have a little bit confusion about building xen from the source
here's what i did
i build xen from the source
git clone git://xenbits.xen.org/xen.git <http://xenbits.xen.org/xen.git>

make dist-xen XEN_TARGET_ARCH=arm32 CROSS_COMPILE=arm-linux-gnueabihf-
CONFIG_EARLY_PRINTK=omap5432

then i download the linux kernel from
git clone
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
<http://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git>

i configured and compiled successfully

i have in my sd card the u-boot.img and MLO and zimage xenuimage and the
file system ubuntu .. it worked fine after some problems .. now i'm
trynig to install linux as domu ..

when i wrote xl list ..the output is no command found ... it looks like
i need to install xen but i don't know how .. i'm really confused .
where should i install it and how
does any body can help me


You will need to compile and install the tools on the board (see [1]).

Regards,

[1] http://wiki.xenproject.org/wiki/Compiling_Xen_From_Source

--
Julien Grall

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


Re: [Xen-devel] [PATCH v2] xen/arm64: correctly emulate the {w, x}zr registers

2016-03-23 Thread Julien Grall

Hi Stefano,

On 22/02/16 17:38, Stefano Stabellini wrote:

On Fri, 15 Jan 2016, Ian Campbell wrote:

I read the patch and looks good to me. You can add my

Reviewed-by: Stefano Stabellini 

I have a couple of minor comments, which you can ignore or address as
you commit the patch.


This patch fell through the crack. Some compilers may generate MMIO 
access with *zr, so we need this patch in Xen 4.7 (and potentially 
backport it).


Are you fine if the patch is not respined?

Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v2] xen/arm64: correctly emulate the {w, x}zr registers

2016-03-24 Thread Julien Grall

(CC committers)

On 24/03/16 10:54, Stefano Stabellini wrote:

On Wed, 23 Mar 2016, Julien Grall wrote:

Hi Stefano,

On 22/02/16 17:38, Stefano Stabellini wrote:

On Fri, 15 Jan 2016, Ian Campbell wrote:

I read the patch and looks good to me. You can add my

Reviewed-by: Stefano Stabellini 

I have a couple of minor comments, which you can ignore or address as
you commit the patch.


This patch fell through the crack. Some compilers may generate MMIO access
with *zr, so we need this patch in Xen 4.7 (and potentially backport it).

Are you fine if the patch is not respined?


Yes, I am fine with it as is.


Thanks. Can someone commit this patch?

Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v2] xen/arm64: correctly emulate the {w, x}zr registers

2016-03-24 Thread Julien Grall

Hi Jan,

On 24/03/16 11:28, Jan Beulich wrote:

On 24.03.16 at 12:12,  wrote:

(CC committers)

On 24/03/16 10:54, Stefano Stabellini wrote:

On Wed, 23 Mar 2016, Julien Grall wrote:

Hi Stefano,

On 22/02/16 17:38, Stefano Stabellini wrote:

On Fri, 15 Jan 2016, Ian Campbell wrote:

I read the patch and looks good to me. You can add my

Reviewed-by: Stefano Stabellini 

I have a couple of minor comments, which you can ignore or address as
you commit the patch.


This patch fell through the crack. Some compilers may generate MMIO access
with *zr, so we need this patch in Xen 4.7 (and potentially backport it).

Are you fine if the patch is not respined?


Yes, I am fine with it as is.


Thanks. Can someone commit this patch?


I think after over two months this should simply be resubmitted
(with the R-b, and possibly with the minor comments addressed).
I for one don't have this in my mailbox anymore, and picking it
up from the xen-devel archive would yield obfuscated email
addresses.


I will do it.

Cheers,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v6 12/22] arm/acpi: Prepare EFI memory descriptor for Dom0

2016-03-24 Thread Julien Grall

Hi Shannon,

On 24/03/16 15:06, Shannon Zhao wrote:

On 2016年03月23日 00:04, Julien Grall wrote:

On 22/03/16 13:16, Shannon Zhao wrote:

On 2016年03月22日 00:51, Julien Grall wrote:

+memory_map[offset].Attribute = EFI_MEMORY_WB;
+}
+
+for( i = 0; i < acpi_mem.nr_banks; i++, offset++ )
+{
+memory_map[offset].Type = EfiACPIReclaimMemory;
+memory_map[offset].PhysicalStart = acpi_mem.bank[i].start;
+memory_map[offset].NumberOfPages =
PFN_UP(acpi_mem.bank[i].size);


Ditto

You are also assuming that acpi_mem.bank[i].size will always be aligned
to 4KB. If so, we may expose unwanted data to the guest.

Based on how the field is set, I would add a BUG_ON to ensure this
condition.

UEFI spec says
"EFI memory descriptors of type EfiACPIReclaimMemory and
EfiACPIMemoryNVS must be aligned on a 4 KiB boundary and must be a
multiple of 4 KiB in size."

So I think the size is aligned to 4kb, right?


Right. I was suggested to add a BUG_ON to document the constraint and
ensure nobody will play with acpi_mem outside EFI.

A such check would also be nice for mem->bank[i].size;

sorry, I didn't get the idea. How to add a BUG_ON to check the size?


Something like:

BUG_ON(acpi_mem.bank[i].size & EFI_PAGE_MASK);

Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v6 13/22] arm/acpi: Map the new created EFI and ACPI tables to Dom0

2016-03-24 Thread Julien Grall

Hi Shannon,

On 24/03/16 14:59, Shannon Zhao wrote:

On 2016年03月23日 00:16, Julien Grall wrote:

On 22/03/16 13:18, Shannon Zhao wrote:

On 2016年03月22日 08:42, Julien Grall wrote:

On 17/03/2016 09:40, Shannon Zhao wrote:

From: Shannon Zhao 

Map the UEFI and ACPI tables which we created to non-RAM space in Dom0.

Signed-off-by: Shannon Zhao 
Reviewed-by: Stefano Stabellini 
---
xen/arch/arm/domain_build.c | 15 +++
1 file changed, 15 insertions(+)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 008fc76..e036887 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1691,6 +1691,21 @@ static int prepare_acpi(struct domain *d,
struct kernel_info *kinfo)
acpi_create_efi_mmap_table(d->arch.efi_acpi_gpa,
d->arch.efi_acpi_len,
   d->arch.efi_acpi_table, &kinfo->mem,
tbl_add);

+/* Map the EFI and ACPI tables to Dom0 */
+rc = map_regions_rw(d,
+paddr_to_pfn(d->arch.efi_acpi_gpa),
+PFN_UP(d->arch.efi_acpi_len),
+
paddr_to_pfn(virt_to_maddr(d->arch.efi_acpi_table)));


The ACPI/EFI tables could potentially have data in the cache but are not
written into the memory (because Xen is mapping the RAM with caching
enabled). However, DOM0 may decide to map it with cache disabled.
Therefore it would be possible for the domain to see wrong data.

So I think you need to clean the cache for this region.

Oh, that would be good. Is there any existing function I can use?


You could reuse p2m_cache_flush. However this function will only flush
cache for p2m_ram_* entries.

I think the best way would be to extend the CACHEFLUSH operations and
maybe p2m_cache_flush (?).

So it needs to extend the CACHEFLUSH case to handle the p2m_mmio_direct
entry, right?


Hmmm thinking a bit more about this solution. It would allow a domain to 
flush device memory region. And we don't want that.


There is actually a simpler solution, you can directly call 
clean_and_invalidate_dcache_va_range on your buffer. I.e


clean_and_invalidate_dcache_va_range(d->arch.efi_acpi_table, 
d->arch.efi_acpi_len);




BTW, does the case you said exist? Before xen switches to Dom0, will it
invalid the cache?


Yes, we had some issue in the past with the kernel, device tree, and 
initramfs passed to a domain. The domains boot with cache disabled, the 
domains will retrieve garbage if the data didn't reach the RAM.


Currently, Xen only flushes some part of the cache associated to the 
data copied in the guest memory (for instance see 
raw_copy_to_guest_flush_dcache).


Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v6 15/22] arm/acpi: Permit access all Xen unused SPIs for Dom0

2016-03-24 Thread Julien Grall

Hi Shannon,

On 24/03/16 15:01, Shannon Zhao wrote:

On 2016年03月23日 02:18, Julien Grall wrote:



+ * Dom0 configures the interrupt, set the interrupt type and
route it to
+ * Dom0.
+ */
+for( i = NR_LOCAL_IRQS; i < vgic_num_irqs(d); i++ )
+{
+desc = irq_to_desc(i);
+if( desc->action != NULL)


Well some of the SPIs used by Xen may not be registered yet. For
instance the SMMU driver doesn't register any SPIs until it's necessary
(i.e a device is assigned to a domain).

But when SMMU requests some SPI, it will delete the route and
reconfigure the SPI, right?


No, the SMMU driver will fail to request the interrupt if it's already 
in use.


Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v6 17/22] arm/gic: Add a new callback to deny Dom0 access to GIC regions

2016-03-24 Thread Julien Grall

Hi Shannon,

On 24/03/16 15:03, Shannon Zhao wrote:

On 2016年03月24日 20:45, Stefano Stabellini wrote:

On Tue, 22 Mar 2016, Julien Grall wrote:

@@ -809,6 +835,10 @@ static u32 gicv2_make_hwdom_madt(const struct domain
*d, u32 offset)
   {
   return 0;
   }
+static int gicv2_iomem_deny_access(const struct domain *d)
+{
+return 0;
+}


I don't see any benefits to have iomem_deny_access only implemented when
CONFIG_ACPI is built.

Because in this case, you will also deny the iomem when Xen is booting using
device tree.

That's true, it would be better to do that for device tree too.


Ok, I'll move it out of the CONFIG_ACPI. But calling it for device tree
would be another patch I think.


Oh right. However my point is this function is not ACPI specific at all. 
So there is no need to compile out when ACPI is disabled.


Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v7 01/22] arm/acpi: Estimate memory required for acpi/efi tables

2016-03-29 Thread Julien Grall

Hi Shannon,

On 25/03/16 13:48, Shannon Zhao wrote:

[...]


+static int estimate_acpi_efi_size(struct domain *d, struct kernel_info *kinfo)
+{
+size_t efi_size, acpi_size, madt_size;
+u64 addr;
+struct acpi_table_rsdp *rsdp_tbl;
+struct acpi_table_header *table;
+
+efi_size = estimate_efi_size(kinfo->mem.nr_banks);
+
+acpi_size = ROUNDUP(sizeof(struct acpi_table_fadt), 8);
+acpi_size += ROUNDUP(sizeof(struct acpi_table_stao), 8);
+
+madt_size = sizeof(struct acpi_table_madt)
++ sizeof(struct acpi_madt_generic_interrupt) * d->max_vcpus
++ sizeof(struct acpi_madt_generic_distributor);
+if ( d->arch.vgic.version == GIC_V3 )
+madt_size += sizeof(struct acpi_madt_generic_redistributor)
+ * d->arch.vgic.nr_regions;
+acpi_size += ROUNDUP(madt_size, 8);
+
+addr = acpi_os_get_root_pointer();
+if ( !addr )
+{
+printk("Unable to get acpi root pointer\n");
+return -EINVAL;
+}
+
+rsdp_tbl = acpi_os_map_memory(addr, sizeof(struct acpi_table_rsdp));
+if ( !rsdp_tbl )
+{
+printk("Unable to map RSDP table\n");
+return -EINVAL;
+}
+
+table = acpi_os_map_memory(rsdp_tbl->xsdt_physical_address,
+   sizeof(struct acpi_table_header));
+if ( !table )


rsdp_tbl will be left mapped if Xen fails to map the XSDT.

As you don't use rsdp_tbl later, I would move 
acpi_os_unmap_memory(rsdp_tlb,...) here.


With this change:

Acked-by: Julien Grall 



+{
+printk("Unable to map XSDT table\n");
+return -EINVAL;
+}
+
+/* Add place for STAO table in XSDT table */
+acpi_size += ROUNDUP(table->length + sizeof(u64), 8);
+acpi_os_unmap_memory(table, sizeof(struct acpi_table_header));
+acpi_os_unmap_memory(rsdp_tbl, sizeof(struct acpi_table_rsdp));
+
+acpi_size += ROUNDUP(sizeof(struct acpi_table_rsdp), 8);
+d->arch.efi_acpi_len = PAGE_ALIGN(ROUNDUP(efi_size, 8)
+  + ROUNDUP(acpi_size, 8));
+
+return 0;
+}


Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v7 02/22] arm/acpi: Add a helper function to get the acpi table offset

2016-03-29 Thread Julien Grall

Hi Shannon,

On 25/03/16 13:48, Shannon Zhao wrote:

These tables are aligned with 64bit.

Signed-off-by: Shannon Zhao 
Reviewed-by: Stefano Stabellini 
---
v7: add commnets to explain what thsi function does
---
  xen/arch/arm/acpi/lib.c| 20 
  xen/include/asm-arm/acpi.h |  2 ++
  2 files changed, 22 insertions(+)

diff --git a/xen/arch/arm/acpi/lib.c b/xen/arch/arm/acpi/lib.c
index db5c4d8..cee2454 100644
--- a/xen/arch/arm/acpi/lib.c
+++ b/xen/arch/arm/acpi/lib.c
@@ -60,3 +60,23 @@ bool_t __init acpi_psci_hvc_present(void)
  {
  return acpi_gbl_FADT.arm_boot_flags & ACPI_FADT_PSCI_USE_HVC;
  }
+
+/*
+ * This function is used to get the offset of some new created ACPI or EFI 
table
+ * in the allocated memory region. Currently the tables should be created in 
the
+ * order of enum EFI_MEM_RES.
+ */


The function could be called after the table is created and still return 
the correct value. I would clearly write in the description when this 
function can be called or not. Something along those lines:


"This function returns the offset of a given ACPI/EFI table in the 
allocated memory region.


Currently, the tables should be created in the same order as their 
associated 'index' in the enum EFI_MEM_RES. This means the function 
won't return the correct offset until all the tables before a given 
'index' are created.".


Also, the description of an external function is usually done in the header.

Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v7 04/22] arm/gic: Add a new callback for creating MADT table for Dom0

2016-03-29 Thread Julien Grall

Hi Shannon,

On 25/03/16 13:48, Shannon Zhao wrote:

Add a new member in gic_hw_operations which is used to create MADT table
for Dom0.

Signed-off-by: Shannon Zhao 
Reviewed-by: Stefano Stabellini 


Acked-by: Julien Grall 

Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v7 09/22] arm/p2m: Add helper functions to map memory regions

2016-03-29 Thread Julien Grall

Hi Shannon,

On 25/03/16 13:48, Shannon Zhao wrote:

From: Parth Dixit 

Create a helper function for mapping with cached attributes and
read-write range.

Signed-off-by: Parth Dixit 
Signed-off-by: Shannon Zhao 
Reviewed-by: Stefano Stabellini 


Acked-by: Julien Grall 

Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v7 10/22] arm/acpi: Map all other tables for Dom0

2016-03-29 Thread Julien Grall

Hi Shannon,

On 25/03/16 13:48, Shannon Zhao wrote:

Map all other ACPI tables into Dom0 using 1:1 mappings.

Signed-off-by: Shannon Zhao 
Reviewed-by: Stefano Stabellini 


Acked-by: Julien Grall 

Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v7 12/22] arm/acpi: Prepare EFI memory descriptor for Dom0

2016-03-29 Thread Julien Grall

Hi Shannon,

On 25/03/16 13:48, Shannon Zhao wrote:

Create EFI memory descriptors to tell Dom0 the RAM region information,
ACPI table regions and EFI tables reserved regions.

Signed-off-by: Parth Dixit 
Signed-off-by: Shannon Zhao 
Reviewed-by: Stefano Stabellini 

Acked-by: Julien Grall 

Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v7 11/22] arm/acpi: Prepare EFI system table for Dom0

2016-03-29 Thread Julien Grall

Hi Shannon,

On 25/03/16 13:48, Shannon Zhao wrote:

Prepare EFI system table for Dom0 to describe the information of UEFI.

Signed-off-by: Parth Dixit 
Signed-off-by: Shannon Zhao 
Reviewed-by: Stefano Stabellini 


Acked-by: Julien Grall 

Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v7 13/22] arm/acpi: Map the new created EFI and ACPI tables to Dom0

2016-03-29 Thread Julien Grall

Hi Shannon,

On 25/03/16 13:48, Shannon Zhao wrote:

Map the UEFI and ACPI tables which we created to non-RAM space in Dom0.

Signed-off-by: Shannon Zhao 
Reviewed-by: Stefano Stabellini 


With the suggestion below:

Acked-by: Julien Grall 


---
v7: flush the cache
---
  xen/arch/arm/domain_build.c | 19 +++
  1 file changed, 19 insertions(+)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 954e0e3..70c8421 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1723,6 +1723,25 @@ static int prepare_acpi(struct domain *d, struct 
kernel_info *kinfo)
  acpi_create_efi_system_table(d, tbl_add);
  acpi_create_efi_mmap_table(d, &kinfo->mem, tbl_add);

+/* Map the EFI and ACPI tables to Dom0 */
+rc = map_regions_rw_cache(d,
+  paddr_to_pfn(d->arch.efi_acpi_gpa),
+  PFN_UP(d->arch.efi_acpi_len),
+  
paddr_to_pfn(virt_to_maddr(d->arch.efi_acpi_table)));
+if ( rc != 0 )
+{
+printk(XENLOG_ERR "Unable to map EFI/ACPI table 0x%"PRIx64
+   " - 0x%"PRIx64" in domain %d\n",
+   d->arch.efi_acpi_gpa & PAGE_MASK,
+   PAGE_ALIGN(d->arch.efi_acpi_gpa + d->arch.efi_acpi_len) - 1,
+   d->domain_id);
+return rc;
+}
+
+/* Flush cache of this region in case Dom0 gets wrong data. */


NIT: I think it would be clearer if you say:

"Flush the cache for this region, otherwise DOM0 may read wrong data 
when the cache is disabled."



+clean_and_invalidate_dcache_va_range(d->arch.efi_acpi_table,
+     d->arch.efi_acpi_len);
+
  return 0;
  }
  #else



Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v7 14/22] arm/acpi: Create min DT stub for Dom0

2016-03-29 Thread Julien Grall

Hi Shannon,

On 25/03/16 13:48, Shannon Zhao wrote:

Create a DT for Dom0 for ACPI-case only. DT contains minimal required
informations such as Dom0 bootargs, initrd, efi description table and


NIT: informations/information/


address of uefi memory table.

Also document this device tree bindings of "hypervisor" and
"hypervisor/uefi" node.

Signed-off-by: Naresh Bhat 
Signed-off-by: Parth Dixit 
Signed-off-by: Shannon Zhao 
Reviewed-by: Stefano Stabellini 


Acked-by: Julien Grall 

Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v7 15/22] arm/acpi: Permit access all Xen unused SPIs for Dom0

2016-03-29 Thread Julien Grall

Hi Shannon,

On 25/03/16 13:48, Shannon Zhao wrote:

Allow DOM0 to use all SPIs but the ones used by Xen. Then when Dom0
configures the interrupt, it could set the interrupt type and route it
to Dom0.

Signed-off-by: Shannon Zhao 
Reviewed-by: Stefano Stabellini 


Acked-by: Julien Grall 

Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v7 16/22] arm/acpi: Configure SPI interrupt type and route to Dom0 dynamically

2016-03-29 Thread Julien Grall

Hi Shannon,

On 25/03/16 13:48, Shannon Zhao wrote:

Interrupt information is described in DSDT and is not available at the
time of booting. Check if the interrupt is permitted to access and set
the interrupt type, route it to guest dynamically only for SPI
and Dom0.

Signed-off-by: Parth Dixit 
Signed-off-by: Shannon Zhao 
Reviewed-by: Stefano Stabellini 


Acked-by: Julien Grall 

Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v7 17/22] arm/gic: Add a new callback to deny Dom0 access to GIC regions

2016-03-29 Thread Julien Grall

Hi Shannon,

On 25/03/16 13:48, Shannon Zhao wrote:

Add a new member in gic_hw_operations which is used to deny Dom0 access
to GIC regions.

Signed-off-by: Shannon Zhao 


Acked-by: Julien Grall 

Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v7 19/22] hvm/params: Add a new delivery type for event-channel in HVM_PARAM_CALLBACK_IRQ

2016-03-29 Thread Julien Grall

Hi Shannon,

On 25/03/16 13:48, Shannon Zhao wrote:

This new delivery type which is for ARM shares the same value with
HVM_PARAM_CALLBACK_TYPE_VECTOR which is for x86.

val[15:8] is flag: val[7:0] is a PPI.
To the flag, bit 8 stands the interrupt mode is edge(1) or level(0) and
bit 9 stands the interrupt polarity is active low(1) or high(0).

Cc: Ian Jackson 
Cc: Jan Beulich 
Cc: Keir Fraser 
Cc: Tim Deegan 
Signed-off-by: Shannon Zhao 


Reviewed-by: Julien Grall 

Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v7 20/22] xen/acpi: Fix event-channel interrupt when booting with ACPI

2016-03-29 Thread Julien Grall

Hi Shannon,

On 25/03/16 13:48, Shannon Zhao wrote:

Store the event-channel interrupt number and flag in HVM parameter
HVM_PARAM_CALLBACK_IRQ. Then Dom0 could get it through hypercall
HVMOP_get_param.

Signed-off-by: Shannon Zhao 
Reviewed-by: Stefano Stabellini 


Acked-by: Julien Grall 

Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v7 22/22] xen/arm64: Add ACPI support

2016-03-29 Thread Julien Grall

Hi Shannon,

On 25/03/16 13:48, Shannon Zhao wrote:

Add ACPI support on arm64 xen hypervisor. Enable EFI support on ARM.

Cc: Jan Beulich 
Signed-off-by: Shannon Zhao 
Acked-by: Jan Beulich 
Reviewed-by: Stefano Stabellini 


Acked-by: Julien Grall 

Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v7 00/22] Prepare UEFI and ACPI tables for Dom0 on ARM64

2016-03-29 Thread Julien Grall

Hi Jan,

On 29/03/16 08:12, Jan Beulich wrote:

On 25.03.16 at 14:48,  wrote:

These patches are Part 4 (and last part) of the previous patch set I
sent which adds ACPI support for arm64 on Xen[1]. Split them as an
individual set for convenient reviewing.

These patches create UEFI and ACPI tables which are mapped to Dom0's
space and add other preparations for Dom0 to use ACPI. Then at last
enable ACPI support on ARM64.


Looks like this series is ready to go in up to patch 20. Julien, you
had a number of comments on v6, and I didn't follow the
discussion too closely - could you (just informally, no need to send
out further individual acks if you didn't mean to send such anyway)
confirm that all the issues you had raised have got taken care of?


I got some comments on patches #1, #2, #13 and #14. The rest is fine by me.

Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v7 03/17] Xen: xlate: Use page_to_xen_pfn instead of page_to_pfn

2016-03-29 Thread Julien Grall

Hi Shannon,

On 24/03/16 14:44, Shannon Zhao wrote:

Make xen_xlate_map_ballooned_pages work with 64K pages. In that case
Kernel pages are 64K in size but Xen pages remain 4K in size. Xen pfns
refer to 4K pages.

Signed-off-by: Shannon Zhao 
Reviewed-by: Stefano Stabellini 
---
  drivers/xen/xlate_mmu.c | 26 --
  1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/xen/xlate_mmu.c b/drivers/xen/xlate_mmu.c
index 9692656..28f728b 100644
--- a/drivers/xen/xlate_mmu.c
+++ b/drivers/xen/xlate_mmu.c
@@ -207,9 +207,12 @@ int __init xen_xlate_map_ballooned_pages(xen_pfn_t **gfns, 
void **virt,
void *vaddr;
int rc;
unsigned int i;
+   unsigned long nr_pages;
+   xen_pfn_t xen_pfn = 0;

BUG_ON(nr_grant_frames == 0);
-   pages = kcalloc(nr_grant_frames, sizeof(pages[0]), GFP_KERNEL);
+   nr_pages = DIV_ROUND_UP(nr_grant_frames, XEN_PFN_PER_PAGE);
+   pages = kcalloc(nr_pages, sizeof(pages[0]), GFP_KERNEL);
if (!pages)
return -ENOMEM;

@@ -218,22 +221,25 @@ int __init xen_xlate_map_ballooned_pages(xen_pfn_t 
**gfns, void **virt,
kfree(pages);
return -ENOMEM;
}
-   rc = alloc_xenballooned_pages(nr_grant_frames, pages);
+   rc = alloc_xenballooned_pages(nr_pages, pages);
if (rc) {
-   pr_warn("%s Couldn't balloon alloc %ld pfns rc:%d\n", __func__,
-   nr_grant_frames, rc);
+   pr_warn("%s Couldn't balloon alloc %ld pages rc:%d\n", __func__,
+   nr_pages, rc);
kfree(pages);
kfree(pfns);
return rc;
}
-   for (i = 0; i < nr_grant_frames; i++)
-   pfns[i] = page_to_pfn(pages[i]);
+   for (i = 0; i < nr_grant_frames; i++) {
+   if ((i % XEN_PFN_PER_PAGE) == 0)
+   xen_pfn = page_to_xen_pfn(pages[i / XEN_PFN_PER_PAGE]);
+   pfns[i] = pfn_to_gfn(xen_pfn++);
+   }


Would it be possible to re-use xen_for_each_gfn? This will avoid 
open-coding the loop to break down the Linux page.


Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v7 10/17] arm/xen: Get event-channel irq through HVM_PARAM when booting with ACPI

2016-03-29 Thread Julien Grall

Hi Shannon,

On 24/03/16 14:44, Shannon Zhao wrote:

When booting with ACPI, it could get the event-channel irq through


The kernel will always get the event-channel IRQ through 
HVM_PARAM_CALLBACK_IRQ.


So I would say: ", the kernel will get the event-channel..."


HVM_PARAM_CALLBACK_IRQ.

Signed-off-by: Shannon Zhao 
Reviewed-by: Stefano Stabellini 
---
  arch/arm/xen/enlighten.c | 36 +++-
  1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index d94f726..680aae0 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -30,6 +30,7 @@
  #include 
  #include 
  #include 
+#include 

  #include 

@@ -278,6 +279,35 @@ void __init xen_early_init(void)
add_preferred_console("hvc", 0, NULL);
  }

+static void __init xen_acpi_guest_init(void)
+{
+#ifdef CONFIG_ACPI
+   struct xen_hvm_param a;
+   int interrupt, trigger, polarity;
+
+   a.domid = DOMID_SELF;
+   a.index = HVM_PARAM_CALLBACK_IRQ;
+   xen_events_irq = 0;
+
+   if (!HYPERVISOR_hvm_op(HVMOP_get_param, &a)) {
+   if ((a.value >> 56) == HVM_PARAM_CALLBACK_TYPE_PPI) {
+   interrupt = a.value & 0xff;
+   trigger = ((a.value >> 8) & 0x1) ? ACPI_EDGE_SENSITIVE
+: ACPI_LEVEL_SENSITIVE;
+   polarity = ((a.value >> 8) & 0x2) ? ACPI_ACTIVE_LOW
+ : ACPI_ACTIVE_HIGH;
+   xen_events_irq = acpi_register_gsi(NULL, interrupt,
+  trigger, polarity);
+   }
+   }


Can you invert the condition to remove one layer of indentation?

Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v7 13/17] ARM: Xen: Document UEFI support on Xen ARM virtual platforms

2016-03-29 Thread Julien Grall

Hi Shannon,

On 24/03/16 14:44, Shannon Zhao wrote:

Add a "uefi" node under /hypervisor node in FDT, then Linux kernel could
scan this to get the UEFI information.

CC: Rob Herring 
Signed-off-by: Shannon Zhao 
Acked-by: Rob Herring 
Reviewed-by: Stefano Stabellini 
---
  Documentation/devicetree/bindings/arm/xen.txt | 33 +++
  1 file changed, 33 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/xen.txt 
b/Documentation/devicetree/bindings/arm/xen.txt
index 0f7b9c2..6f83f76 100644
--- a/Documentation/devicetree/bindings/arm/xen.txt
+++ b/Documentation/devicetree/bindings/arm/xen.txt
@@ -15,6 +15,26 @@ the following properties:
  - interrupts: the interrupt used by Xen to inject event notifications.
A GIC node is also required.


You need to update the recent of the document based on the changes you 
made in the Xen one. See [1].



+To support UEFI on Xen ARM virtual platforms, Xen populates the FDT "uefi" node
+under /hypervisor with following parameters:
+
+
+Name  | Size   | Description
+
+xen,uefi-system-table | 64-bit | Guest physical address of the UEFI System
+ || Table.
+
+xen,uefi-mmap-start   | 64-bit | Guest physical address of the UEFI memory
+ || map.
+
+xen,uefi-mmap-size| 32-bit | Size in bytes of the UEFI memory map
+  || pointed to in previous entry.
+
+xen,uefi-mmap-desc-size   | 32-bit | Size in bytes of each entry in the UEFI
+  || memory map.
+
+xen,uefi-mmap-desc-ver| 32-bit | Version of the mmap descriptor format.
+

  Example (assuming #address-cells = <2> and #size-cells = <2>):

@@ -22,4 +42,17 @@ hypervisor {
compatible = "xen,xen-4.3", "xen,xen";
reg = <0 0xb000 0 0x2>;
interrupts = <1 15 0xf08>;
+   uefi {
+   xen,uefi-system-table = <0x>;
+   xen,uefi-mmap-start = <0x>;
+   xen,uefi-mmap-size = <0x>;
+   xen,uefi-mmap-desc-size = <0x>;
+   xen,uefi-mmap-desc-ver = <0x>;
+};
  };
+
+The format and meaning of the "xen,uefi-*" parameters are similar to those in
+Documentation/arm/uefi.txt, which are provided by the regular UEFI stub. 
However
+they differ because they are provided by the Xen hypervisor, together with a 
set
+of UEFI runtime services implemented via hypercalls, see
+http://xenbits.xen.org/docs/unstable/hypercall/x86_64/include,public,platform.h.html.



Regards,

[1] 
http://lists.xenproject.org/archives/html/xen-devel/2016-03/msg03413.html


--
Julien Grall

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


Re: [Xen-devel] ARMv8: New board bring up hangs in kernel start?

2016-03-29 Thread Julien Grall

On 23/03/16 17:24, Dirk Behme wrote:

Hi,


Hello Dirk,

Sorry for the late answer.


trying to bring up Xen on a new ARMv8 64-bit Cortex A57 eval board, I
get [1] and then its hanging there.


The logs look normal.

Do you know where the kernel get stuck? You can press CTRL-a 3 times to 
get access to the Xen console and then press:

* 0 => dump Dom0 registers
* d => dump registers


I'd guess that it hangs due to missing timer interrupt, maybe missing
interrupts at all?

Any hints how to debug this? Or where to look?

It might be possible that the board's firmware (arm-trusted-firmware
based) doesn't configure anything correctly. Firmware is running at EL3,
Xen at EL2. The same kernel is running fine without Xen.

Using a JTAG debugger I've put breakpoints into xen/arch/arm/time.c
timer_interrupt() & vtimer_interrupt() but these don't seem to be called
at all (?)


They should be called if the timer is configured correctly.


Best regards

Dirk

[1]


[...]

> (XEN) Checking for initrd in /chosen
> (XEN) RAM: 4800 - 7fff
> (XEN)
> (XEN) MODULE[0]: 4800 - 480058a2 Device Tree
> (XEN) MODULE[1]: 4820 - 48c0 Kernel
> (XEN)
> (XEN) Command line: console=dtuart dom0_mem=512M loglvl=all
> (XEN) Placing Xen at 0x7fe0-0x8000
> (XEN) Update BOOTMOD_XEN from 4900-49112e01 =>
> 7fe0-7ff12e01
> (XEN) Domain heap initialised
> (XEN) Platform: ARMv8 Cortex A57 64-bit eval board
> (XEN) Taking dtuart configuration from /chosen/stdout-path
> (XEN) Looking for dtuart at "/soc/serial@e6e88000", options ""
>   Xen 4.7-unstable
> (XEN) Xen version 4.7-unstable (dirk@build) (aarch64-poky-linux-gcc
> (Linaro GCC 4.9-2015.03) 4.9.3 20150311 (prerelease)) debug=y Mon Mar 21
> 09:15:03 CET 2016
> (XEN) Latest ChangeSet: Tue Feb 9 09:37:15 2016 +0100 git:b0a2893

I can't find this changeset in tree. Can you provide your baseline 
commit and the list of patches you applied on top?


Also have you tried a newer version of Xen?

Regards,

--
Julien Grall

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


Re: [Xen-devel] Running Xen on Nvidia Jetson-TK1

2016-03-29 Thread Julien Grall

Hello Dushyant,

On 24/03/16 11:05, Dushyant Behl wrote:

I was not receiving the dom0 logs because of a mistake in my dom0
bootargs. In the bootargs the option
for earlyprintk was not marked as Xen. Now that I've enabled it I'm
able to see some bootlog from dom0 linux.

At least now I'm able to figure out the reason of Linux running into
infinite loop.

It seems like Linux is not receiving any interrupts from the arch
timer and when it tries
to calibrate the timer delay then there's a loop where linux waits to
receive ticks to calculate
loops_per_jiffies and that's the point where dom0 is running into the
infinite loop.
(exact point is http://osxr.org:8080/linux/source/init/calibrate.c#0196)

This is the dom0 bootlog which I received after correcting the
earlyprintk argument -


Can you provide the full log? So we can see if there is anything which 
could give us a hint about your problem.




(XEN) DOM0: Uncompressing Linux... done, booting the kernel.
(XEN) DOM0: [0.00] Booting Linux on physical CPU 0x0
(XEN) DOM0: [0.00] Initializing cgroup subsys cpu
(XEN) DOM0: [0.00] Initializing cgroup subsys cpuacct
(XEN) DOM0: [0.00] Linux version
4.1.0-196898-g2e68ed9-dirty(root@ubuntu-server) (gcc version 4.7.3
(Ubuntu/Linaro 4.7.3-11ubuntu
(XEN) DOM0: 1) ) #12 SMP PREEMPT Thu Mar 24 09:56:36 UTC 2016
(XEN) DOM0: [0.00] CPU: ARMv7 Processor [413fc0f3] revision 3
(ARMv7), cr=30c5387d
(XEN) DOM0: [0.00] CPU: PIPT / VIPT nonaliasing data cache,
PIPT instruction cache
(XEN) DOM0: [0.00] Machine model: NVIDIA Tegra124 Jetson TK1
(XEN) DOM0: [0.00] bootconsole [earlycon0] enabled
(XEN) DOM0: [0.00] cma: Reserved 64 MiB at 0xbc00
(XEN) DOM0: [0.00] Forcing write-allocate cache policy for SMP
(XEN) DOM0: [0.00] Memory policy: Data cache writealloc
(XEN) DOM0: [0.00] psci: probing for conduit method from DT.
(XEN) DOM0: [0.00] psci: PSCIv0.2 detected in firmware.
(XEN) DOM0: [0.00] psci: Using standard PSCI v0.2 function IDs
(XEN) DOM0: [0.00] PERCPU: Embedded 12 pages/cpu @dbb77000
s19712 r8192 d21248 u49152
(XEN) DOM0: [0.00] Built 1 zonelists in Zone order, mobility
grouping on.  Total pages: 130048
(XEN) DOM0: [0.00] Kernel command line: console=hvc0
console=tty1 earlyprintk=xen root=/dev/mmcblk0p1 rw rootwait
tegraid=40.1.1.0.0
(XEN) DOM0: [0.00] PID hash table entries: 2048 (order: 1, 8192 bytes)
(XEN) DOM0: [0.00] Dentry cache hash table entries: 65536
(order: 6, 262144 bytes)
(XEN) DOM0: [0.00] Inode-cache hash table entries: 32768
(order: 5, 131072 bytes)
(XEN) DOM0: [0.00] Memory: 441884K/524288K available (7657K
kernel code, 634K rwdata, 2584K rodata, 484K init, 383K bss, 16868K
reserved, 65536K cma-reserved, 0K highmem)
(XEN) DOM0: [0.00] Virtual kernel memory layout:
(XEN) DOM0: [0.00] vector  : 0x - 0x1000   (   4 kB)
(XEN) DOM0: [0.00] fixmap  : 0xffc0 - 0xfff0   (3072 kB)
(XEN) DOM0: [0.00] vmalloc : 0xe080 - 0xff00   ( 488 MB)
(XEN) DOM0: [0.00] lowmem  : 0xc000 - 0xe000   ( 512 MB)
(XEN) DOM0: [0.00] pkmap   : 0xbfe0 - 0xc000   (   2 MB)
(XEN) DOM0: [0.00] modules : 0xbf00 - 0xbfe0   (  14 MB)
(XEN) DOM0: [0.00]   .text : 0xc0008000 - 0xc0a08c10   (10244 kB)
(XEN) DOM0: [0.00]   .init : 0xc0a09000 - 0xc0a82000   ( 484 kB)
(XEN) DOM0: [0.00]   .data : 0xc0a82000 - 0xc0b20bec   ( 635 kB)
(XEN) DOM0: [0.00].bss : 0xc0b23000 - 0xc0b82ea0   ( 384 kB)
(XEN) DOM0: [0.00] Preemptible hierarchical RCU implementation.
(XEN) DOM0: [0.00] Build-time adjustment of leaf fanout to 32.
(XEN) DOM0: [0.00] NR_IRQS:16 nr_irqs:16 16
(XEN) DOM0: [0.00] of_irq_init: children remain, but no parents
(XEN) DOM0: [0.00] L2C: failed to init: -19
(XEN) DOM0: [0.00] irq: no irq domain found for /interrupt-controller !
(XEN) DOM0: [0.00] irq: no irq domain found for /interrupt-controller !
(XEN) DOM0: [0.00] irq: no irq domain found for /interrupt-controller !
(XEN) DOM0: [0.00] arch_timer: No interrupt available, giving up
(XEN) DOM0: [0.00] sched_clock: 32 bits at 100 Hz, resolution
1000ns, wraps every 2147483647500ns
(XEN) DOM0: [0.00] Console: colour dummy device 80x30
(XEN) DOM0: [0.00] console [tty1] enabled

Can anyone explain why Linux is not able to get any interrupts from
the arch timer?
Is this some problem with Xen's interrupt mappings or some issue with
the dom0 kernel?


From the log: "arch_timer: No interrupt available, giving up". So the 
kernel is not able to get the interrupt from device tree.

Which device-tree are you using for the board?

Regards,

--
Julien Grall


Re: [Xen-devel] Question about /proc/interrupts on Xen ARM.

2016-03-29 Thread Julien Grall

On 24/03/16 12:20, 신정섭 wrote:

HI I have a question Question about '/proc/interrupts' on Xen ARM.


Hello,

Please avoid to attach image on the mailing list.


I'm running Xen ARM 4.4.2 on Arndale Board.
I Attached a Image that is is result of 'cat /proc/interrupts' DomainU on Xen 
ARM.

I know that, event channel on Xen ARM only use the IRQ 31.
But xenbus, hvc_console, blk, eth0 use IRQ 32 ~ 38 not IRQ 31.

So I want to know event channel on Xen ARM not only use IRQ 31?


In attached Image

1. When Domain0 want to block data to DomainU,
Domain0 write block data in blk I/O ring and Inject IRQ 34 instead of IRQ 31?

2. When Domain0 wants to block data to DomainU,
Domain0 write block data in blk I/O ring and Inject IRQ 31.
After Injection IRQ 31, event channel driver in DomainU checks blk
I/O ring and increases count of IRQ 34 in /proc/interrupt?

Now i'm confusing...


The first number of each line is the IRQ number. It's a number made by 
Linux and may not match the physical interrupt number.


The IRQ 31 (i.e GIC 31) is the actual PPI used to notify the domain for 
new events.


The IRQs 32-38 are bound to event channels.

I hope this help you.

Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v7 03/17] Xen: xlate: Use page_to_xen_pfn instead of page_to_pfn

2016-03-30 Thread Julien Grall

Hi Shannon,

On 30/03/16 08:38, Shannon Zhao wrote:



On 2016/3/30 0:28, Julien Grall wrote:

On 24/03/16 14:44, Shannon Zhao wrote:

Make xen_xlate_map_ballooned_pages work with 64K pages. In that case
Kernel pages are 64K in size but Xen pages remain 4K in size. Xen pfns
refer to 4K pages.

Signed-off-by: Shannon Zhao 
Reviewed-by: Stefano Stabellini 
---
   drivers/xen/xlate_mmu.c | 26 --
   1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/xen/xlate_mmu.c b/drivers/xen/xlate_mmu.c
index 9692656..28f728b 100644
--- a/drivers/xen/xlate_mmu.c
+++ b/drivers/xen/xlate_mmu.c
@@ -207,9 +207,12 @@ int __init
xen_xlate_map_ballooned_pages(xen_pfn_t **gfns, void **virt,
   void *vaddr;
   int rc;
   unsigned int i;
+unsigned long nr_pages;
+xen_pfn_t xen_pfn = 0;

   BUG_ON(nr_grant_frames == 0);
-pages = kcalloc(nr_grant_frames, sizeof(pages[0]), GFP_KERNEL);
+nr_pages = DIV_ROUND_UP(nr_grant_frames, XEN_PFN_PER_PAGE);
+pages = kcalloc(nr_pages, sizeof(pages[0]), GFP_KERNEL);
   if (!pages)
   return -ENOMEM;

@@ -218,22 +221,25 @@ int __init
xen_xlate_map_ballooned_pages(xen_pfn_t **gfns, void **virt,
   kfree(pages);
   return -ENOMEM;
   }
-rc = alloc_xenballooned_pages(nr_grant_frames, pages);
+rc = alloc_xenballooned_pages(nr_pages, pages);
   if (rc) {
-pr_warn("%s Couldn't balloon alloc %ld pfns rc:%d\n", __func__,
-nr_grant_frames, rc);
+pr_warn("%s Couldn't balloon alloc %ld pages rc:%d\n", __func__,
+nr_pages, rc);
   kfree(pages);
   kfree(pfns);
   return rc;
   }
-for (i = 0; i < nr_grant_frames; i++)
-pfns[i] = page_to_pfn(pages[i]);
+for (i = 0; i < nr_grant_frames; i++) {
+if ((i % XEN_PFN_PER_PAGE) == 0)
+xen_pfn = page_to_xen_pfn(pages[i / XEN_PFN_PER_PAGE]);
+pfns[i] = pfn_to_gfn(xen_pfn++);
+}


Would it be possible to re-use xen_for_each_gfn? This will avoid
open-coding the loop to break down the Linux page.

I don't think so. Using xen_acpi_guest_init will require factoring
"pfns[i] = pfn_to_gfn(xen_pfn++)" to a function with parameter pfns[i].
How can we pass pfns[i]?


By using the variable data. Something along those lines:

struct map_balloon_pages
{
  xen_pfn_t *pfns;
  unsigned int idx;
};

static void setup_balloon_gfn(unsigned long gfn, void *data)
{
  struct map_balloon_pages *info = data;


  data->pfns[data->idx] = gfn;
  data->idx++;
}

And then in xen_xlate_map_ballooned_pages

xen_for_each_gfn(pages, nr_grant_frames, setup_balloon_gfn, &data);

Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v8 00/21] Prepare UEFI and ACPI tables for Dom0 on ARM64

2016-03-30 Thread Julien Grall

Hi,

On 30/03/16 11:07, Shannon Zhao wrote:

From: Shannon Zhao 

These patches are Part 4 (and last part) of the previous patch set I
sent which adds ACPI support for arm64 on Xen[1]. Split them as an
individual set for convenient reviewing.

These patches create UEFI and ACPI tables which are mapped to Dom0's
space and add other preparations for Dom0 to use ACPI. Then at last
enable ACPI support on ARM64.

See individual patch for changes.


For the rest of the patches:

Acked-by: Julien Grall 

Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH] arm64: xen_boot: Fix xen boot using Grub on AARCH64

2016-03-30 Thread Julien Grall

Hello,

Ping?

Regards,

On 19/02/16 16:28, Julien Grall wrote:

Xen is currently crashing because of malformed compatible property for
the boot module. This is because the property string is not
null-terminated as requested by the ePAR spec.
---
  grub-core/loader/arm64/xen_boot.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/grub-core/loader/arm64/xen_boot.c 
b/grub-core/loader/arm64/xen_boot.c
index a914eb8..8ae43d7 100644
--- a/grub-core/loader/arm64/xen_boot.c
+++ b/grub-core/loader/arm64/xen_boot.c
@@ -156,7 +156,7 @@ prepare_xen_module_params (struct xen_boot_binary *module, 
void *xen_boot_fdt)
grub_fdt_add_subnode (xen_boot_fdt, chosen_node, module_name);

retval = grub_fdt_set_prop (xen_boot_fdt, module_node, "compatible",
- MODULE_CUSTOM_COMPATIBLE, 
sizeof(MODULE_CUSTOM_COMPATIBLE) - 1);
+ MODULE_CUSTOM_COMPATIBLE, 
sizeof(MODULE_CUSTOM_COMPATIBLE));
if (retval)
  return grub_error (GRUB_ERR_IO, "failed to update FDT");




--
Julien Grall

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


Re: [Xen-devel] [PATCH v8 20/21] xen/arm: Add a hypercall for device mmio mapping

2016-03-30 Thread Julien Grall

Hi,

On 30/03/16 17:11, Konrad Rzeszutek Wilk wrote:

On Wed, Mar 30, 2016 at 06:08:13PM +0800, Shannon Zhao wrote:

From: Shannon Zhao 

It needs to map platform or amba device mmio to Dom0 on ARM. But when
booting with ACPI, it can't get the mmio region in Xen due to lack of
AML interpreter to parse DSDT table. Therefore, let Dom0 call a
hypercall to map mmio region when it adds the devices.

Here we add a new map space like the XEN_DOMCTL_memory_mapping to map
mmio region for Dom0. Also add a helper to combine the
xsm_add_to_physmap and XENMAPSPACE_dev_mmio space check together.

Cc: Ian Jackson 
Cc: Jan Beulich 
Cc: Keir Fraser 
Cc: Tim Deegan 
Signed-off-by: Shannon Zhao 
---
v8: add a helper to combine xsm_add_to_physmap and XENMAPSPACE_dev_mmio
space check together


I was by accident reviewing the earlier ersion. So let me give comments
here as well.
.. snipp.


Jan already applied the patch series to xengit/staging. Shannon, can you 
send a follow-up patch to fix at least the printk?



+int map_dev_mmio_region(struct domain *d,
+unsigned long start_gfn,
+unsigned long nr,
+unsigned long mfn)
+{
+int res;
+
+if ( !(nr && iomem_access_permitted(d, start_gfn, start_gfn + nr - 1)) )
+return 0;
+
+res = map_mmio_regions(d, start_gfn, nr, mfn);
+if ( res < 0 )
+{
+printk(XENLOG_ERR "Unable to map [%#lx - %#lx] in Dom%d\n",


Should this be printk ratelimited?


I think so. Today the domain can only be the hardware domain but it may 
change in the future.



+   start_gfn, start_gfn + nr - 1, d->domain_id);
+return res;
+}
+
+return 0;
+}
+
  int guest_physmap_add_entry(struct domain *d,
  unsigned long gpfn,
  unsigned long mfn,


Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH 3/6] xentrace: P2M lookup suport for ARM platform

2016-03-30 Thread Julien Grall

On 28/03/16 19:55, Ben Sanda wrote:

Julien and George,


Hi Ben,

Sorry for the late answer.


Thank you for the comments. I had one question I wanted to ask.


A DOMID_XEN page could be read only too. For instance, the meta-data
of the trace buffer is read-only (see t_info), we don't want a domain
to be able to overwrite them.



However, all the foreign page are mapped read-write. You will need to
rework the code to map a foreign domain (see
XENMAPSPACE_gmfn_foreign) to allow read-only foreign page (maybe by
adding a new p2m_type_t?).


I understand what you are saying in general, but I'm not familiar
enough with the Xen memory mapping system to know how to actually
implement this. The p2m_type_t p2m_ram_ro exists, which I could assign
to read-only pages, but I'm unsure as to how to detect whether a
request is to a read only mapping or a read-write. The normal (non
DOMID_XEN) p2m_lookup function normally does this by reading the root-
level page tables and somehow extracting the mapping type from the
lpae_t structure. Given that we are not looking up the page tables for
non-translated addresses, I'm not sure where/how to find the correct
mapping type. Can I still lookup the page table entries for the MFN
address and extract the p2m_type_t the same way?


You can know if the page is writable by looking to the page field 
u.inuse.type_info (PGT_writable_page means the page is writable).


Regards,

--
Julien Grall

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


Re: [Xen-devel] Running Xen on Nvidia Jetson-TK1

2016-04-01 Thread Julien Grall

Hello Dushyant,

On 29/03/16 21:56, Dushyant Behl wrote:

On Wed, Mar 30, 2016 at 12:31 AM, Julien Grall  wrote:

On 24/03/16 11:05, Dushyant Behl wrote:



(XEN) DOM0: [0.00] irq: no irq domain found for /interrupt-controller !
(XEN) DOM0: [0.00] irq: no irq domain found for /interrupt-controller !
(XEN) DOM0: [0.00] irq: no irq domain found for /interrupt-controller !
(XEN) DOM0: [0.00] arch_timer: No interrupt available, giving up


It looks like to me that Xen is not recreating the device-tree 
correctly. I would look into the kernel to find what is expected.


Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v2] xen/arm: use XENLOG_G_ERR instead

2016-04-01 Thread Julien Grall

Hi,

On 01/04/16 15:26, Jan Beulich wrote:

On 31.03.16 at 11:41,  wrote:

From: Shannon Zhao 

To distinguish the error logs in function map_dev_mmio_region, use
XENLOG_G_ERR instead of XENLOG_ERR.


Both title and description are pretty strange.


You are right. Shannon, I would say:

"xen/arm: map_dev_mmio_region: printk should be ratelimited

The function map_dev_mmio_region is used in a hypercall. Therefore all 
printks should be ratelimited to avoid a malicious guest flooding the 
console."


Regards,

--
Julien Grall

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


Re: [Xen-devel] [TESTDAY] Test report of ARM CubieTruck with Xen 4.7 (staging)

2016-04-01 Thread Julien Grall

Hi Konrad,

Thank you for testing Xen ARM on the cubietruck!

On 30/03/16 16:30, Konrad Rzeszutek Wilk wrote:

*Hardware: ARM CubieTruck A20 (2GB).

*Software:  Linaro 14.04 with
  - Xen 4.7 (829e03c acpi: drop CONFIG_ACPI_BOOT and use CONFIG_ACPI instead + 
xsplice.v5)
  - Linux v4.5 (with revert of 88f8b1bb41c6208f81b6a480244533ded7b59493 "
stmmac: Fix 'eth0: No PHY found' regression")
  - Linux v4.4
  - Linux v4.3
  - Linux v4.2

*Guest operating systems: Ubuntu 14.04

*Functionality tested: ARM Manual Smoke Test

*Comments:

I get these:

(XEN) Freed 260kB init memory.
(XEN) d0v0: vGICD: unhandled word write 0x to ICACTIVER4
(XEN) d0v0: vGICD: unhandled word write 0x to ICACTIVER8
(XEN) d0v0: vGICD: unhandled word write 0x to ICACTIVER12
(XEN) d0v0: vGICD: unhandled word write 0x to ICACTIVER16
(XEN) d0v0: vGICD: unhandled word write 0x to ICACTIVER0
(XEN) d0v1: vGICD: unhandled word write 0x to ICACTIVER0
(XEN) d1v0: vGICD: unhandled word write 0x to ICACTIVER0


Xen doesn't currently emulate the registers I*ACTIVERn. For now, it's 
not a big problem as the usage of I*ACTIVER0 is very limited. This will 
need to be supported properly when migration will be added for ARM.


Regards,

--
Julien Grall

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


Re: [Xen-devel] ARMv8: New board bring up hangs in kernel start?

2016-04-01 Thread Julien Grall



On 31/03/16 18:41, Dirk Behme wrote:

Hello Julien,


Hello Dirk,


On 29.03.2016 20:53, Julien Grall wrote:

On 23/03/16 17:24, Dirk Behme wrote:

trying to bring up Xen on a new ARMv8 64-bit Cortex A57 eval board, I
get [1] and then its hanging there.


The logs look normal.

Do you know where the kernel get stuck? You can press CTRL-a 3 times
to get access to the Xen console and then press:
* 0 => dump Dom0 registers
* d => dump registers



Hmm, CTRL-a 3 times doesn't seem to work, either.

This does need working interrupts, too? I.e. that it doesn't work is an
additional hint that anything with the interrupt handling might be wrong?


The entry point for all the interrupts is do_IRQ. You can add a 
breakpoint there to know if you receive interrupts.




Maybe I should debug this, first.

It's handled by Xen's drivers/char/console.c / serial.c and the board
specific UART device driver, correct?


The generic IRQ code (see do_IRQ) will dispatch the interrupt directly
to the interrupt handler you specific via setup_irq/request_irq.
Usually this handler is specific to the driver.


I'd guess that it hangs due to missing timer interrupt, maybe missing
interrupts at all?

Any hints how to debug this? Or where to look?

It might be possible that the board's firmware (arm-trusted-firmware
based) doesn't configure anything correctly. Firmware is running at
EL3,
Xen at EL2. The same kernel is running fine without Xen.

Using a JTAG debugger I've put breakpoints into xen/arch/arm/time.c
timer_interrupt() & vtimer_interrupt() but these don't seem to be
called
at all (?)


They should be called if the timer is configured correctly.


Best regards

Dirk

[1]


[...]

 > (XEN) Checking for initrd in /chosen
 > (XEN) RAM: 4800 - 7fff
 > (XEN)
 > (XEN) MODULE[0]: 4800 - 480058a2 Device Tree
 > (XEN) MODULE[1]: 4820 - 48c0 Kernel
 > (XEN)
 > (XEN) Command line: console=dtuart dom0_mem=512M loglvl=all
 > (XEN) Placing Xen at 0x7fe0-0x8000
 > (XEN) Update BOOTMOD_XEN from 4900-49112e01 =>
 > 7fe0-7ff12e01
 > (XEN) Domain heap initialised
 > (XEN) Platform: ARMv8 Cortex A57 64-bit eval board
 > (XEN) Taking dtuart configuration from /chosen/stdout-path
 > (XEN) Looking for dtuart at "/soc/serial@e6e88000", options ""
 >   Xen 4.7-unstable
 > (XEN) Xen version 4.7-unstable (dirk@build) (aarch64-poky-linux-gcc
 > (Linaro GCC 4.9-2015.03) 4.9.3 20150311 (prerelease)) debug=y Mon
Mar 21
 > 09:15:03 CET 2016
 > (XEN) Latest ChangeSet: Tue Feb 9 09:37:15 2016 +0100 git:b0a2893

I can't find this changeset in tree. Can you provide your baseline
commit and the list of patches you applied on top?



This is

483ad4439f7fc7 xen-access: minor fixes

plus a local patch to support the board's serial port.


Is it a patch to add earlyprintk or a completely new driver?





Also have you tried a newer version of Xen?



I've switched to the recent master

a6f2cdb63 x86/hvm/viridian: keep APIC assist page mapped

now. No difference.

I'll have a deeper look into the interrupt configuration.

Is there anywhere some basic description which interrupts are supposed
to be handled by XEN and which by the Linux kernel? I.e. how the ARM GIC
should be configured regarding the distributor/CPU/virtual parts?


All the interrupts are taken by Xen. The function do_IRQ in Xen will 
dispatch the IRQ either to a guest or call a Xen specific handler.


Xen handles only a limited number of interrupt:
* timers
* UART
* SMMU

The rest is either routed to guests or blacklisted by Xen.

Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v3] xen/arm64: check XSM Magic and Signature from the second unknown module.

2016-04-01 Thread Julien Grall
default: break;
  }
+if (kind_guess > 1 && check_xsm_signature(fdt, node, name,
+  address_cells, size_cells))
+        kind = BOOTMOD_XSM;
  }

  prop = fdt_get_property(fdt, node, "reg", &len);



Regards,

--
Julien Grall

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


Re: [Xen-devel] xen-utils not running within xen or no compatible utils

2016-04-04 Thread Julien Grall

Hello,

On 01/04/2016 17:47, Wei Liu wrote:

Add back xen-devel

On Fri, Apr 01, 2016 at 05:45:31PM +0200, aicha hamza wrote:

yes i compiled xen from the source  .. this is exactly what i did
http://wiki.xenproject.org/wiki/Xen_ARM_with_Virtualization_Extensions/OMAP5432_uEVM

and i get xen running in my board .. but i can't build guest because there


So this is done. Good. I misread.


is no xen-utils-common
and as i told you before there is internet connection in my board .. so how
can i get sen.utils package !!



As far as I can tell xen-tools uses Debian debootstrap to build a guest
which means you have to have Internet connection. Maybe you can prebuild
the image from somewhere.

Julien, do you have pointers on how to get an ARM guest image?


Linaro provides pre-built image [1]. Note that I haven't tried it recently.

As mentioned by Wei, Debootstrap is a good alternative. You will get a 
small rootfs ready in a couple of minutes.


Regards,

[1] https://releases.linaro.org/15.06/ubuntu/vexpress/

--
Julien Grall

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


Re: [Xen-devel] xen-utils not running within xen or no compatible utils

2016-04-04 Thread Julien Grall

(Adding back xen-devel)

Please don't top post and keep xen-devel CCed.

On 04/04/2016 14:15, aicha hamza wrote:

i'm actually looking for building a dom u for omap5 but the problem that
i don't have an internet connection .. so i can't get the xen-utils ..
(no xl command) is there an other method to have a guest for my board


The first step would be looking why the board doesn't get network.

Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH V2 0/9] xentrace/xenalyze support on ARM

2016-04-04 Thread Julien Grall

Hello,

On 04/04/2016 15:11, Wei Liu wrote:

On Fri, Apr 01, 2016 at 04:33:44PM -0400, Benjamin Sanda wrote:

---
Changed since v1:
   * Removed Flask changes as deemed uncessesary and unclear in
 purpose
   * Corrected all commit messages to be line limited to 72 chars
   * Implimented v1 review comments as indicated in each file's
 commit log.

Benjamin Sanda (9):
   xenalyze: Support for ARM platform
   xentrace: Support for ARM platform
   xentrace: Support for ARM platform
   xentrace: Support for ARM platform
   xentrace: Support for ARM platform
   xentrace: Support for ARM platform
   xentrace: Support for ARM platform
   xentrace: Support for ARM platform
   xentrace: Support for ARM platform


You patches all have the same subject line.  Please make them more
specific. See my reply to #1 for example.


+1

Also, you should at least check that Xen still builds after applying 
each patch. Ideally, you also need to be careful to not break any 
feature currently supported. It's useful when someone needs to bisect 
the tree.


For instance, you use the function get_pg_owner for ARM in patch #2 but 
introduce the function in patch #4. This will break ARM build. So the 
patch #2 should be moved after #4.


Furthermore, you remove the functions get_pg_owner and put_pg_owner for 
x86 in patch #3 and then re-introduced them in patch #4. Therefore, the 
x86 will be broken after #3. In this case, it's better to have a patch 
that move the 2 functions from x86 to common.


Finally, please CC all the x86 maintainers (Jan and Andrew) on x86 
changes. You need to manually check the CCs as under certain conditions 
the script get_maintainers.pl may not return all the relevant maintainers.


I will wait the next iteration of this series to review the code.

Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH] xen: Add comment for missing FROZEN notifier transitions

2016-04-04 Thread Julien Grall

(CC Stefano new e-mail address)

Hello Anna-Maria,

On 04/04/2016 13:32, Anna-Maria Gleixner wrote:

Xen guests do not offline/online CPUs during suspend/resume and
therefore FROZEN notifier transitions are not required. Add this
explanation as a comment in the code to get not confused why
CPU_TASKS_FROZEN masked transitions are not considered.

Cc: David Vrabel 
Cc: Stefano Stabellini 
Cc: xen-de...@lists.xenproject.org
Signed-off-by: Anna-Maria Gleixner 
---
  arch/arm/xen/enlighten.c |6 ++
  arch/x86/xen/enlighten.c |7 +++
  drivers/xen/events/events_fifo.c |6 ++
  3 files changed, 19 insertions(+)

--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -213,6 +213,12 @@ static int xen_cpu_notification(struct n
unsigned long action,
void *hcpu)
  {
+   /*
+* Xen guests do not offline/online CPUs during
+* suspend/resume, thus CPU_TASKS_FROZEN masked transitions
+* are not considered.
+*/
+
switch (action) {
case CPU_STARTING:
xen_percpu_init();
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1788,6 +1788,13 @@ static int xen_hvm_cpu_notify(struct not
  void *hcpu)
  {
int cpu = (long)hcpu;
+
+   /*
+* Xen guests do not offline/online CPUs during
+* suspend/resume, thus CPU_TASKS_FROZEN masked transitions
+* are not considered.
+*/
+
switch (action) {
case CPU_UP_PREPARE:
xen_vcpu_setup(cpu);
--- a/drivers/xen/events/events_fifo.c
+++ b/drivers/xen/events/events_fifo.c
@@ -425,6 +425,12 @@ static int evtchn_fifo_cpu_notification(
int cpu = (long)hcpu;
int ret = 0;

+   /*
+* Xen guests do not offline/online CPUs during
+* suspend/resume, thus CPU_TASKS_FROZEN masked transitions
+* are not considered.
+   */


NIT: The '*' is not aligned with the others.


+
switch (action) {
case CPU_UP_PREPARE:
if (!per_cpu(cpu_control_block, cpu))



Regards,

--
Julien Grall

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


Re: [Xen-devel] ARMv8: New board bring up hangs in kernel start?

2016-04-06 Thread Julien Grall



On 04/04/2016 16:44, Dirk Behme wrote:

Hi Julien,


Hello Dirk,


On 01.04.2016 18:34, Julien Grall wrote:



On 31/03/16 18:41, Dirk Behme wrote:

Also have you tried a newer version of Xen?



I've switched to the recent master

a6f2cdb63 x86/hvm/viridian: keep APIC assist page mapped

now. No difference.

I'll have a deeper look into the interrupt configuration.

Is there anywhere some basic description which interrupts are supposed
to be handled by XEN and which by the Linux kernel? I.e. how the ARM
GIC
should be configured regarding the distributor/CPU/virtual parts?


All the interrupts are taken by Xen. The function do_IRQ in Xen will
dispatch the IRQ either to a guest or call a Xen specific handler.

Xen handles only a limited number of interrupt:
 * timers
 * UART
 * SMMU

The rest is either routed to guests or blacklisted by Xen.



Ok, thanks, that helps :) Once I have it working, maybe I post a patch
to add this info to the documentation.


That would be good. Thank you!



Just an other question:

On ARMv8 64-bit Xen is supposed to be started at EL2 *nonsecure*, correct?


That's right.



It looks to me that the GICv2 on my board is already partly configured
by the firmware at secure EL3. That does mean, whatever
gicv2_dist_init() and gicv2_cpu_init() are supposed to do, they can't do
it (completely) because they don't have access to the secure part of the
GIC (?)


Which is normal, the secure part of the GIC should have already been 
initialized by the firmware running at secure EL3.


Do you know if Linux was able to initialize KVM on baremetal?

Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v10 00/17] Add ACPI support for Xen Dom0 on ARM64

2016-04-06 Thread Julien Grall

Hi Shannon,

On 01/04/2016 16:48, Shannon Zhao wrote:

This patch set adds ACPI support for Xen Dom0 on ARM64. The relevant Xen
ACPI on ARM64 design document could be found from [1].

This patch set adds a new FDT node "uefi" under /hypervisor to pass UEFI
information. Introduce a bus notifier of AMBA and Platform bus to map
the new added device's MMIO space. Make Xen domain use
xlated_setup_gnttab_pages to setup grant table and a new hypercall to
get event-channel irq.

Regarding the initialization flow of Linux kernel, it needs to move
xen_early_init() before efi_init(). Then xen_early_init() will check
whether it runs on Xen through the /hypervisor node and efi_init() will
call a new function fdt_find_xen_uefi_params(), to parse those
xen,uefi-* parameters just like the existing efi_get_fdt_params().

And in arm64_enable_runtime_services() it will check whether it runs on
Xen and call another new function xen_efi_runtime_setup() to setup
runtime service instead of efi_native_runtime_setup(). The
xen_efi_runtime_setup() will assign the runtime function pointers with
the functions of driver/xen/efi.c.

And since we pass a /hypervisor node and a /chosen node to Dom0, it
needs to check whether the DTS only contains a /hypervisor node and a
/chosen node in acpi_boot_table_init().

Patches are tested on FVP base model. They can be fetched from[2].


I have tested this series and Linux is booting up to the prompt:

Tested-by: Julien Grall 

Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v10 02/17] xen/grant-table: Move xlated_setup_gnttab_pages to common place

2016-04-06 Thread Julien Grall

Hi Shannon,

On 01/04/2016 16:49, Shannon Zhao wrote:

Move xlated_setup_gnttab_pages to common place, so it can be reused by
ARM to setup grant table.

Rename it to xen_xlate_map_ballooned_pages.

Signed-off-by: Shannon Zhao 
Reviewed-by: Stefano Stabellini 


Reviewed-by: Julien Grall 

Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v10 03/17] Xen: xlate: Use page_to_xen_pfn instead of page_to_pfn

2016-04-06 Thread Julien Grall

Hi Shannon,

On 01/04/2016 16:49, Shannon Zhao wrote:

Make xen_xlate_map_ballooned_pages work with 64K pages. In that case
Kernel pages are 64K in size but Xen pages remain 4K in size. Xen pfns
refer to 4K pages.

Signed-off-by: Shannon Zhao 
Reviewed-by: Stefano Stabellini 


Reviewed-by: Julien Grall 

Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v10 04/17] arm/xen: Use xen_xlate_map_ballooned_pages to setup grant table

2016-04-06 Thread Julien Grall

Hi Shannon,

On 01/04/2016 16:49, Shannon Zhao wrote:

Use xen_xlate_map_ballooned_pages to setup grant table. Then it doesn't
rely on DT or ACPI to pass the start address and size of grant table.

Signed-off-by: Shannon Zhao 
Acked-by: Stefano Stabellini 


Reviewed-by: Julien Grall 

Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v10 05/17] xen: memory : Add new XENMAPSPACE type XENMAPSPACE_dev_mmio

2016-04-06 Thread Julien Grall

Hi Shannon,

On 01/04/2016 16:49, Shannon Zhao wrote:

Add a new type of Xen map space for Dom0 to map device's MMIO region.

Signed-off-by: Shannon Zhao 


Reviewed-by: Julien Grall 

Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v10 06/17] Xen: ARM: Add support for mapping platform device mmio

2016-04-06 Thread Julien Grall
nt r = 0;
+
+   if (pdev->num_resources == 0 || pdev->resource == NULL)
+   return NOTIFY_OK;
+
+   switch (action) {
+   case BUS_NOTIFY_ADD_DEVICE:
+   r = xen_map_device_mmio(pdev->resource, pdev->num_resources);
+   break;
+   case BUS_NOTIFY_DEL_DEVICE:
+   r = xen_unmap_device_mmio(pdev->resource, pdev->num_resources);
+   break;
+   default:
+   return NOTIFY_DONE;
+   }
+   if (r)
+   dev_err(&pdev->dev, "Platform: Failed to %s device %s MMIO!\n",
+   action == BUS_NOTIFY_ADD_DEVICE ? "map" :
+   (action == BUS_NOTIFY_DEL_DEVICE ? "unmap" : "?"),
+   pdev->name);
+
+   return NOTIFY_OK;
+}
+
+static struct notifier_block platform_device_nb = {
+   .notifier_call = xen_platform_notifier,
+};
+
+static int __init register_xen_platform_notifier(void)
+{
+   if (!xen_initial_domain() || acpi_disabled)
+   return 0;
+
+   return bus_register_notifier(&platform_bus_type, &platform_device_nb);
+}
+
+arch_initcall(register_xen_platform_notifier);



Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v10 07/17] Xen: ARM: Add support for mapping AMBA device mmio

2016-04-06 Thread Julien Grall

Hi Shannon,

On 01/04/2016 16:49, Shannon Zhao wrote:

Add a bus_notifier for AMBA bus device in order to map the device
mmio regions when DOM0 booting with ACPI.

Signed-off-by: Shannon Zhao 
Reviewed-by: Stefano Stabellini 


Reviewed-by: Julien Grall 

Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v10 09/17] xen/hvm/params: Add a new delivery type for event-channel in HVM_PARAM_CALLBACK_IRQ

2016-04-06 Thread Julien Grall

Hi Shannon,

On 01/04/2016 16:49, Shannon Zhao wrote:

This new delivery type which is for ARM shares the same value with
HVM_PARAM_CALLBACK_TYPE_VECTOR which is for x86.

val[15:8] is flag: val[7:0] is a PPI.
To the flag, bit 8 stands the interrupt mode is edge(1) or level(0) and
bit 9 stands the interrupt polarity is active low(1) or high(0).

Signed-off-by: Shannon Zhao 
Acked-by: Stefano Stabellini 


Reviewed-by: Julien Grall 

Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v10 10/17] arm/xen: Get event-channel irq through HVM_PARAM when booting with ACPI

2016-04-06 Thread Julien Grall

Hi Shannon,

On 01/04/2016 16:49, Shannon Zhao wrote:

The kernel will get the event-channel IRQ through
HVM_PARAM_CALLBACK_IRQ.

Signed-off-by: Shannon Zhao 
Reviewed-by: Stefano Stabellini 


Reviewed-by: Julien Grall 

Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v10 11/17] ARM: XEN: Move xen_early_init() before efi_init()

2016-04-06 Thread Julien Grall

Hi Shannon,

On 01/04/2016 16:49, Shannon Zhao wrote:

Move xen_early_init() before efi_init(), then when calling efi_init()
could initialize Xen specific UEFI.

Check if it runs on Xen hypervisor through the flat dts.

Cc: Russell King 
Signed-off-by: Shannon Zhao 
Reviewed-by: Stefano Stabellini 


Reviewed-by: Julien Grall 

Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v3] xen/arm: map_dev_mmio_region: printk should be ratelimited

2016-04-07 Thread Julien Grall

Hi Shannon,

On 07/04/16 07:28, Shannon Zhao wrote:

From: Shannon Zhao 

The function map_dev_mmio_region is used in a hypercall. Therefore all
printks should be ratelimited to avoid a malicious guest flooding the
console.

Signed-off-by: Shannon Zhao 
Reviewed-by: Konrad Rzeszutek Wilk 


Acked-by: Julien Grall 

Regards,


---
v3: update commit message
---
  xen/arch/arm/p2m.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 0011708..db21433 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1284,7 +1284,7 @@ int map_dev_mmio_region(struct domain *d,
  res = map_mmio_regions(d, start_gfn, nr, mfn);
  if ( res < 0 )
  {
-printk(XENLOG_ERR "Unable to map [%#lx - %#lx] in Dom%d\n",
+printk(XENLOG_G_ERR "Unable to map [%#lx - %#lx] in Dom%d\n",
 start_gfn, start_gfn + nr - 1, d->domain_id);
  return res;
  }



--
Julien Grall

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


Re: [Xen-devel] [PATCH v10 06/17] Xen: ARM: Add support for mapping platform device mmio

2016-04-07 Thread Julien Grall

Hi Shannon,

On 07/04/16 02:37, Shannon Zhao wrote:



On 2016/4/6 20:16, Julien Grall wrote:

+gpfns[j] = XEN_PFN_DOWN(r->start) + j;
+idxs[j] = XEN_PFN_DOWN(r->start) + j;
+}
+
+xatp.domid = DOMID_SELF;
+xatp.size = nr;
+xatp.space = XENMAPSPACE_dev_mmio;
+
+set_xen_guest_handle(xatp.gpfns, gpfns);
+set_xen_guest_handle(xatp.idxs, idxs);
+set_xen_guest_handle(xatp.errs, errs);
+
+rc = HYPERVISOR_memory_op(XENMEM_add_to_physmap_range, &xatp);
+kfree(gpfns);
+kfree(idxs);
+kfree(errs);
+if (rc)
+return rc;


Shouldn't we redo the mapping if the hypercall fails?

Hmm, why? If it fails again when we redo the mapping, what should we do
then? Redo again?


Because the device MMIO region is left half mapped in DOM0 address space.

After having another look to your patch, if an error occurs, the 
notifier will still return NOTIFY_OK. This will lead to random data 
abort when the driver is accessing the MMIO regions as the device will 
be considered fully functional.


However, even if the notifier return NOTIFY_BAD, the function device_add 
doesn't care about the return value of blocking_notifier_call_chain. I 
think this need to be fixed.



I think if it fails at the first time it will always fail no matter how
many times we do.


I was speaking about the mappings that succeeded. They will unlikely 
fail during removal. If they ever fail you can just ignore the error.


Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v10 00/17] Add ACPI support for Xen Dom0 on ARM64

2016-04-07 Thread Julien Grall

On 07/04/16 02:39, Shannon Zhao wrote:

Hi Julien,


Hi Shannon,


On 2016/4/6 19:32, Julien Grall wrote:

Hi Shannon,

On 01/04/2016 16:48, Shannon Zhao wrote:

This patch set adds ACPI support for Xen Dom0 on ARM64. The relevant Xen
ACPI on ARM64 design document could be found from [1].

This patch set adds a new FDT node "uefi" under /hypervisor to pass UEFI
information. Introduce a bus notifier of AMBA and Platform bus to map
the new added device's MMIO space. Make Xen domain use
xlated_setup_gnttab_pages to setup grant table and a new hypercall to
get event-channel irq.

Regarding the initialization flow of Linux kernel, it needs to move
xen_early_init() before efi_init(). Then xen_early_init() will check
whether it runs on Xen through the /hypervisor node and efi_init() will
call a new function fdt_find_xen_uefi_params(), to parse those
xen,uefi-* parameters just like the existing efi_get_fdt_params().

And in arm64_enable_runtime_services() it will check whether it runs on
Xen and call another new function xen_efi_runtime_setup() to setup
runtime service instead of efi_native_runtime_setup(). The
xen_efi_runtime_setup() will assign the runtime function pointers with
the functions of driver/xen/efi.c.

And since we pass a /hypervisor node and a /chosen node to Dom0, it
needs to check whether the DTS only contains a /hypervisor node and a
/chosen node in acpi_boot_table_init().

Patches are tested on FVP base model. They can be fetched from[2].


I have tested this series and Linux is booting up to the prompt:

Tested-by: Julien Grall 

Thanks a lot. There are several patches which you didn't give your
comments. So I assume you will review them. If so, I'll wait and update
this series later.


I don't have any comments on those patches. You can go ahead to update 
the patch series.


Regards,

--
Julien Grall

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


[Xen-devel] [for-4.7] xen/arm64: correctly emulate the {w, x}zr registers

2016-04-07 Thread Julien Grall
On AArch64, encoding 31 for an R in the HSR is used to represent
either {w,x}sp or {w,x}zr (See C1.2.4 in ARM DDI 0486A.d) depending on
how the register field is interpreted by the instruction.

All the instructions trapped by Xen (either via a sysreg access or
data abort) interpret encoding 31 as {w,x}zr. Therefore we don't have
to worry about the possibility that a trap could refer to sp or about
decoding the instruction.

For example AArch64 LDR and STR can have zr in the source/target
register , but never sp. sp can be present in the destination
pointer( i.e.  "[sp]"), but that would be represented by the value of
FAR_EL2, not in the HSR.

For AArch32 it is possible for a LDR to target the PC, but this would
not result in a valid ISS in the HSR register. However this could only
occur if loading or storing the PC to MMIO, which we simply choose not
to support for now.

Finally, features such as xenaccess can lead to us trapping on
arbitrary instructions accessing RAM and not just for MMIO. However in
many such cases HSR.ISS is not valid and in general features such as
xenaccess do not rely on the nature of the specific instruction, they
resolve the fault (via information found elsewhere e.g. FAR_EL2)
without needing to know anything about the instruction which triggered
the trap.

The register zr represents the zero register, i.e it will always
return 0 and write to it is ignored. To properly handle this property,
2 new helpers have been introduced {get,set}_user_reg to read/write a
value from/to a register. All the calls to select_user_reg have been
replaced by these 2 helpers.

Furthermore, the code to emulate encoding 31 in select_user_reg has been
dropped because it was invalid. For Aarch64 context, the encoding is
used for sp or zr. For AArch32 context, the ISS won't be valid for data
abort from AArch32 using r15 (i.e pc) as source/destination (See D7-1881
ARM DDI 0487A.d, note the validity is more restrictive than on ARMv7).
It's also not possible to use r15 in co-processor instructions.

This patch fixes setting MMIO register and sysreg to a random value
(actually PC) instead of zero by something like:

*((volatile int*)reg) = 0;

compilers tend to generate "str wzr, [xx]" here.

[ian: added BUG_ON to select_user_reg and clarified bits of the commit message]
Reported-by: Marc Zyngier 
Signed-off-by: Julien Grall 
Signed-off-by: Ian Campbell 
Reviewed-by: Stefano Stabellini 

---

Stefano, let me know the new helper corresponds to change you requested
(see [1])

This patch is a bug fix for Xen 4.7. Without it, a MMIO register and
sysreg will be set to a random value (actually PC) when the zero
register is used.

I'm not sure if we should consider this patch to be backported to Xen
4.6 and Xen 4.5. It depends on other patches and it would require some
rework to backport it alone.

[1] http://lists.xenproject.org/archives/html/xen-devel/2016-02/msg03100.html
---
 xen/arch/arm/io.c  |  34 
 xen/arch/arm/traps.c   | 126 ++---
 xen/arch/arm/vgic-v3.c |   3 +-
 xen/arch/arm/vtimer.c  |  59 -
 xen/include/asm-arm/regs.h |   7 +--
 5 files changed, 158 insertions(+), 71 deletions(-)

diff --git a/xen/arch/arm/io.c b/xen/arch/arm/io.c
index 7e29943..0156755 100644
--- a/xen/arch/arm/io.c
+++ b/xen/arch/arm/io.c
@@ -24,12 +24,19 @@
 #include 
 
 static int handle_read(const struct mmio_handler *handler, struct vcpu *v,
-   mmio_info_t *info, register_t *r)
+   mmio_info_t *info)
 {
 const struct hsr_dabt dabt = info->dabt;
+struct cpu_user_regs *regs = guest_cpu_user_regs();
+/*
+ * Initialize to zero to avoid leaking data if there is an
+ * implementation error in the emulation (such as not correctly
+ * setting r).
+ */
+register_t r = 0;
 uint8_t size = (1 << dabt.size) * 8;
 
-if ( !handler->ops->read(v, info, r, handler->priv) )
+if ( !handler->ops->read(v, info, &r, handler->priv) )
 return 0;
 
 /*
@@ -37,7 +44,7 @@ static int handle_read(const struct mmio_handler *handler, 
struct vcpu *v,
  * Note that we expect the read handler to have zeroed the bits
  * outside the requested access size.
  */
-if ( dabt.sign && (*r & (1UL << (size - 1))) )
+if ( dabt.sign && (r & (1UL << (size - 1))) )
 {
 /*
  * We are relying on register_t using the same as
@@ -45,21 +52,30 @@ static int handle_read(const struct mmio_handler *handler, 
struct vcpu *v,
  * code smaller.
  */
 BUILD_BUG_ON(sizeof(register_t) != sizeof(unsigned long));
-*r |= (~0UL) << size;
+r |= (~0UL) << size;
 }
 
+set_user_reg(regs, dabt.reg, r);
+
 return 1;
 }
 
+static int handle_write(const struct mmio_handler *handler, struct vcpu *v,
+

[Xen-devel] [for-4.7 3/5] xen/arm: acpi: Fix SMP support when booting with ACPI

2016-04-07 Thread Julien Grall
The variable enabled_cpus is used to know the number of CPU enabled in
the MADT.

Currently this variable is used to check the validity of the boot CPU.
It will be considered invalid when "enabled_cpus > 1".

However, this condition also means that multiple CPUs are present on the
system. So secondary will never be brought up.

The correct way to check the validity of the boot CPU is to use the
variable bootcpu_valid.

Signed-off-by: Julien Grall 
---
 xen/arch/arm/acpi/boot.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/arm/acpi/boot.c b/xen/arch/arm/acpi/boot.c
index 2a71660..fd29bdc 100644
--- a/xen/arch/arm/acpi/boot.c
+++ b/xen/arch/arm/acpi/boot.c
@@ -149,7 +149,7 @@ void __init acpi_smp_init_cpus(void)
 return;
 }
 
-if ( enabled_cpus > 1 )
+if ( !bootcpu_valid )
 {
 printk("MADT missing boot CPU MPIDR, not enabling secondaries\n");
 return;
-- 
1.9.1


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


[Xen-devel] [for-4.7 5/5] xen/arm: acpi: Print more error messages in acpi_map_gic_cpu_interface

2016-04-07 Thread Julien Grall
It's helpful to spot any error without having to modify the hypervisor
code.

Signed-off-by: Julien Grall 
---
 xen/arch/arm/acpi/boot.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/xen/arch/arm/acpi/boot.c b/xen/arch/arm/acpi/boot.c
index 602ab39..23285f7 100644
--- a/xen/arch/arm/acpi/boot.c
+++ b/xen/arch/arm/acpi/boot.c
@@ -63,7 +63,10 @@ acpi_map_gic_cpu_interface(struct 
acpi_madt_generic_interrupt *processor)
 
 total_cpus++;
 if ( !enabled )
+{
+printk("Skipping disabled CPU entry with 0x%"PRIx64" MPIDR\n", mpidr);
 return;
+}
 
 if ( enabled_cpus >=  NR_CPUS )
 {
@@ -101,7 +104,11 @@ acpi_map_gic_cpu_interface(struct 
acpi_madt_generic_interrupt *processor)
 }
 
 if ( !acpi_psci_present() )
+{
+printk("PSCI not present, skipping CPU MPIDR 0x%"PRIx64"\n",
+   mpidr);
 return;
+}
 
 if ( (rc = arch_cpu_init(enabled_cpus, NULL)) < 0 )
 {
-- 
1.9.1


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


[Xen-devel] [for-4.7 4/5] xen/arm: acpi: Remove uncessary check in acpi_map_gic_cpu_interface

2016-04-07 Thread Julien Grall
This part of the code will never be executed when the entry
corresponds to the boot CPU.

Also print an error message rather when arch_cpu_init has failed.

Signed-off-by: Julien Grall 
---
 xen/arch/arm/acpi/boot.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/xen/arch/arm/acpi/boot.c b/xen/arch/arm/acpi/boot.c
index fd29bdc..602ab39 100644
--- a/xen/arch/arm/acpi/boot.c
+++ b/xen/arch/arm/acpi/boot.c
@@ -51,6 +51,7 @@ static void __init
 acpi_map_gic_cpu_interface(struct acpi_madt_generic_interrupt *processor)
 {
 int i;
+int rc;
 u64 mpidr = processor->arm_mpidr & MPIDR_HWID_MASK;
 bool_t enabled = !!(processor->flags & ACPI_MADT_ENABLED);
 
@@ -102,16 +103,16 @@ acpi_map_gic_cpu_interface(struct 
acpi_madt_generic_interrupt *processor)
 if ( !acpi_psci_present() )
 return;
 
-/* CPU 0 was already initialized */
-if ( enabled_cpus )
+if ( (rc = arch_cpu_init(enabled_cpus, NULL)) < 0 )
 {
-if ( arch_cpu_init(enabled_cpus, NULL) < 0 )
-return;
-
-/* map the logical cpu id to cpu MPIDR */
-cpu_logical_map(enabled_cpus) = mpidr;
+printk("cpu%d: init failed (0x%"PRIx64" MPIDR): %d\n",
+   enabled_cpus, mpidr, rc);
+return;
 }
 
+/* map the logical cpu id to cpu MPIDR */
+cpu_logical_map(enabled_cpus) = mpidr;
+
 enabled_cpus++;
 }
 
-- 
1.9.1


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


[Xen-devel] [for-4.7 1/5] drivers/pl011: ACPI: The interrupt should always be high level triggered

2016-04-07 Thread Julien Grall
The SPCR does not specify if the interrupt is edge or level triggered.
So the configuration needs to be hardcoded in the code.

Based on the PL011 TRM (see 2.2.8 in ARM DDI 0183G), the interrupt generated
will be active high. This wording implies the interrupt should be high level
triggered. Note that a rising edge triggered interrupt would be described as
"high going edge".

Signed-off-by: Julien Grall 
---
 xen/drivers/char/pl011.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
index fa22edf..88d8488 100644
--- a/xen/drivers/char/pl011.c
+++ b/xen/drivers/char/pl011.c
@@ -327,7 +327,7 @@ static int __init pl011_acpi_uart_init(const void *data)
 }
 
 /* trigger/polarity information is not available in spcr */
-irq_set_type(spcr->interrupt, IRQ_TYPE_EDGE_BOTH);
+irq_set_type(spcr->interrupt, IRQ_TYPE_LEVEL_MASK);
 
 res = pl011_uart_init(spcr->interrupt, spcr->serial_port.address,
   PAGE_SIZE);
-- 
1.9.1


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


[Xen-devel] [for-4.7 2/5] xen/arm: acpi: The boot CPU does not always match the first entry in the MADT

2016-04-07 Thread Julien Grall
Since the ACPI 6.0 errata document [1], the first entry in the MADT
does not have to correspond to the boot CPU.

Introduce a new variable to know if a MADT entry matching the boot CPU
is found. Furthermore, it's not necessary to check if the MPIDR is
duplicated for the boot CPU. So the rest of the function can be skipped.

[1] 1380 Unnecessary restrictions to FW vendors in ordering of GIC structures
in MADT

Signed-off-by: Julien Grall 
---
 xen/arch/arm/acpi/boot.c | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/acpi/boot.c b/xen/arch/arm/acpi/boot.c
index 859aa86..2a71660 100644
--- a/xen/arch/arm/acpi/boot.c
+++ b/xen/arch/arm/acpi/boot.c
@@ -37,7 +37,8 @@
 #include 
 
 /* Processors with enabled flag and sane MPIDR */
-static unsigned int enabled_cpus;
+static unsigned int enabled_cpus = 1;
+static bool __initdata bootcpu_valid;
 
 /* total number of cpus in this system */
 static unsigned int __initdata total_cpus;
@@ -71,10 +72,15 @@ acpi_map_gic_cpu_interface(struct 
acpi_madt_generic_interrupt *processor)
 }
 
 /* Check if GICC structure of boot CPU is available in the MADT */
-if ( (enabled_cpus == 0) && (cpu_logical_map(0) != mpidr) )
+if ( cpu_logical_map(0) == mpidr )
 {
-printk("Firmware bug, invalid CPU MPIDR for cpu0: 0x%"PRIx64" in 
MADT\n",
-   mpidr);
+if ( bootcpu_valid )
+{
+printk("Firmware bug, duplicate boot CPU MPIDR: 0x%"PRIx64" in 
MADT\n",
+   mpidr);
+return;
+}
+bootcpu_valid = true;
 return;
 }
 
-- 
1.9.1


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


[Xen-devel] [for-4.7 0/5] xen/arm: acpi: Bunch of fixes to use ACPI with SMP and PL011

2016-04-07 Thread Julien Grall
Hello,

This patch series fixes secondary bring up and the use of the PL011 UART driver
when Xen boots using ACPI.

Regards,

Cc: wei.l...@citrix.com

Julien Grall (5):
  drivers/pl011: ACPI: The interrupt should always be high level
triggered
  xen/arm: acpi: The boot CPU does not always match the first entry in
the MADT
  xen/arm: acpi: Fix SMP support when booting with ACPI
  xen/arm: acpi: Remove uncessary check in acpi_map_gic_cpu_interface
  xen/arm: acpi: Print more error messages in acpi_map_gic_cpu_interface

 xen/arch/arm/acpi/boot.c | 38 ++
 xen/drivers/char/pl011.c |  2 +-
 2 files changed, 27 insertions(+), 13 deletions(-)

-- 
1.9.1


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


Re: [Xen-devel] [for-4.7 1/5] drivers/pl011: ACPI: The interrupt should always be high level triggered

2016-04-07 Thread Julien Grall

Hi Shannon,

Thank you for the review.

On 07/04/16 13:30, Shannon Zhao wrote:



On 2016/4/7 18:59, Julien Grall wrote:

The SPCR does not specify if the interrupt is edge or level triggered.
So the configuration needs to be hardcoded in the code.

Based on the PL011 TRM (see 2.2.8 in ARM DDI 0183G), the interrupt generated
will be active high. This wording implies the interrupt should be high level
triggered.

I think active high can stand rising edge triggered for edge triggered
interrupt.

E.g. see "Table 5-118 Flag Definitions: Virtual Timer, EL2 timers, and
Secure & Non-Secure EL1 timers" in ACPI SPEC 6.0.


I've spoken with multiple person about the wording and the consensus is 
"active high" would imply high level triggered. So it's very ambiguous.


However, the PL011 is always using a high level triggered. You can look 
at the device tree bindings such as the one for the foundation model.


Also, the SBSA (section 4.3.2 in ARM-DEN-0029 v2.3) states the PL011 
implemented with a level triggered interrupt.


Note, I wasn't able to get the serial console working on my platform 
with edge triggered interrupt.


Regards,

--
Julien Grall

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


Re: [Xen-devel] [for-4.7 1/5] drivers/pl011: ACPI: The interrupt should always be high level triggered

2016-04-07 Thread Julien Grall



On 07/04/16 14:57, Shannon Zhao wrote:

On 2016年04月07日 21:41, Julien Grall wrote:


On 07/04/16 13:30, Shannon Zhao wrote:



On 2016/4/7 18:59, Julien Grall wrote:

The SPCR does not specify if the interrupt is edge or level triggered.
So the configuration needs to be hardcoded in the code.

Based on the PL011 TRM (see 2.2.8 in ARM DDI 0183G), the interrupt
generated
will be active high. This wording implies the interrupt should be
high level
triggered.

I think active high can stand rising edge triggered for edge triggered
interrupt.

E.g. see "Table 5-118 Flag Definitions: Virtual Timer, EL2 timers, and
Secure & Non-Secure EL1 timers" in ACPI SPEC 6.0.


I've spoken with multiple person about the wording and the consensus is
"active high" would imply high level triggered. So it's very ambiguous.

However, the PL011 is always using a high level triggered. You can look
at the device tree bindings such as the one for the foundation model.

Also, the SBSA (section 4.3.2 in ARM-DEN-0029 v2.3) states the PL011
implemented with a level triggered interrupt.

Note, I wasn't able to get the serial console working on my platform
with edge triggered interrupt.


So how about IRQ_TYPE_LEVEL_HIGH instead of IRQ_TYPE_LEVEL_MASK?


Good point. I will likely resend only this patch and update the commit 
message too.


Regards,

--
Julien Grall

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


Re: [Xen-devel] [for-4.7] xen/arm64: correctly emulate the {w, x}zr registers

2016-04-07 Thread Julien Grall

Hi Jan,

On 07/04/2016 18:18, Jan Beulich wrote:

On 07.04.16 at 12:53,  wrote:

---

Stefano, let me know the new helper corresponds to change you requested
(see [1])

This patch is a bug fix for Xen 4.7. Without it, a MMIO register and
sysreg will be set to a random value (actually PC) when the zero
register is used.

I'm not sure if we should consider this patch to be backported to Xen
4.6 and Xen 4.5. It depends on other patches and it would require some
rework to backport it alone.

[1]
http://lists.xenproject.org/archives/html/xen-devel/2016-02/msg03100.html


So the tags and alike would suggest this is ready to be committed,
but the lack of a version number or version history don't really
help support this. Could you please clarify the state of this patch?


Sorry I forgot to mention the changes I made. This is a resend with the 
modification suggested by Stefano.


I've kept Stefano's reviewed-by as he was fine with and without the 
helpers. Although, I would like to give Stefano a chance to object. Can 
you wait a bit before committing?


Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v3] arm: Fix asynchronous aborts (SError exceptions) due to bogus PTEs

2016-04-08 Thread Julien Grall



On 06/04/16 19:57, Shanker Donthineni wrote:

Hi Julien/Stefano,


Hi Shanker,



Any other comments to be addressed? Please propose an alternative
solution to fix the problem if this patch changes are not appropriate.


All the comments have been addressed.



On 03/28/2016 11:46 PM, Shanker Donthineni wrote:

From: Vikram Sethi 

ARMv8 architecture allows performing prefetch data/instructions
from memory locations marked as normal memory. Prefetch does not
mean that the data/instruction has to be used/executed in code
flow. All PTEs that appear to be valid to MMU must contain valid
physical address with proper attributes otherwise MMU table walk
might cause imprecise asynchronous aborts.

The way current XEN code is preparing page tables for frametable
and xenheap memory can create bogus PTEs. This patch fixes the
issue by clearing page table memory before populating EL2 L0/L1
PTEs. Without this patch XEN crashes on Qualcomm Technologies
server chips due to asynchronous aborts.

The speculative/prefetch feature explanation is scattered everywhere
in ARM specification but below two sections have useful information.

E2.8 Memory types and attributes (ver DDI0487A_h)
G4.12.6 External abort on a translation table walk (ver DDI0487A_h)

Signed-off-by: Vikram Sethi 
Signed-off-by: Shanker Donthineni 


Acked-by: Julien Grall 

Regards,

--
Julien Grall

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


Re: [Xen-devel] Running Xen on Nvidia Jetson-TK1

2016-04-08 Thread Julien Grall



On 07/04/16 08:48, Dushyant Behl wrote:

Hello,


Hi Dushyant,


On Fri, Apr 1, 2016 at 3:34 PM, Julien Grall  wrote:

Hello Dushyant,

On 29/03/16 21:56, Dushyant Behl wrote:


On Wed, Mar 30, 2016 at 12:31 AM, Julien Grall 
wrote:


On 24/03/16 11:05, Dushyant Behl wrote:




(XEN) DOM0: [0.00] irq: no irq domain found for
/interrupt-controller !
(XEN) DOM0: [0.00] irq: no irq domain found for
/interrupt-controller !
(XEN) DOM0: [0.00] irq: no irq domain found for
/interrupt-controller !
(XEN) DOM0: [0.00] arch_timer: No interrupt available, giving up



It looks like to me that Xen is not recreating the device-tree correctly. I
would look into the kernel to find what is expected.


This looks like a possible bug (or some missing feature) in Xen's
device tree creation which could
take some time to handle, so if I could be of any more help to you
with this issue please let me know.


There was a conversation on #xen-arm few days ago about this problem.
Xen doesn't correctly recreate the GIC node which result in a loop 
between the interrupt controller. Can you try the below patch?


http://dev.ktemkin.com/misc/xenarm-gic-parents.patch



[I've cc'ed Ian Campbell in this mail (Sorry for cc'ing you explicitly)]


Ian's citrix e-mail is not valid anymore. I have CC'ed the new one.


Ian,

Actually, I want to run Xen on the Tegra Jetson board for some project
of mine but currently Linux-4.1 is
failing as dom0 because its not able to receive interrupts from the arch_timer.
This link contains the dom0 failure boot log -
http://lists.xenproject.org/archives/html/xen-devel/2016-03/msg03715.html

In your patch for *Hacky* support for Jetsok-TK1 you said that you
were able to run guests on
Jetson-tk1 board with Xen. Can I know which kernel version you used as
dom0 (and possibly domU guests)?


Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v3 2/5] xentrace: Memory/Page Mapping support for DOMID_XEN on ARM

2016-04-08 Thread Julien Grall

(CC Stefano's new e-mail address)

Hello Benjamin,

On 04/04/16 19:48, Benjamin Sanda wrote:

  xen/arch/arm/mm.c  |  3 ++-
  xen/arch/arm/p2m.c | 35 +++
  2 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 81f9e2e..19d6c2c 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1099,7 +1099,8 @@ int xenmem_add_to_physmap_one(
  {
  struct domain *od;
  p2m_type_t p2mt;
-od = rcu_lock_domain_by_any_id(foreign_domid);
+od = get_pg_owner(foreign_domid);
+


Please also replace the call to rcu_unlock_domain by put_pg_owner to 
stay consistent.



  if ( od == NULL )
  return -ESRCH;

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index a2a9c4b..a99b670 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -227,11 +227,38 @@ paddr_t p2m_lookup(struct domain *d, paddr_t paddr, 
p2m_type_t *t)
  {
  paddr_t ret;
  struct p2m_domain *p2m = &d->arch.p2m;
+struct page_info *page;
+unsigned long mfn;

-spin_lock(&p2m->lock);
-ret = __p2m_lookup(d, paddr, t);
-spin_unlock(&p2m->lock);
-
+/*
+* DOMID_XEN is considered a PV guest on x86 (i.e MFN == GFN), but
+* on ARM there is no such concept. Thus requests to DOMID_XEN
+* on ARM use a MFN address directly and do not need translation
+* from PFN.
+*/


The coding style for the comment should be:

/*
 * FOo
 * Bar
 */


+if(DOMID_XEN != d->domain_id)


if ( ... )


+{
+spin_lock(&p2m->lock);
+ret = __p2m_lookup(d, paddr, t);
+spin_unlock(&p2m->lock);
+}
+else
+{
+/* retrieve the page to determine read/write or read only mapping */
+mfn = paddr >> PAGE_SHIFT;
+if (mfn_valid(mfn))
+{
+page = mfn_to_page(mfn);
+*t = (page->u.inuse.type_info == PGT_writable_page ?
+p2m_ram_rw : p2m_ram_ro);


Unfortunately, xenmem_add_to_physmap_one will ignore the return type and 
will always map using the type p2m_map_foreign. I would introduce

a new type p2m_map_foreign_ro to allow read-only foreign mapping.

I've looked at the x86 code (p2m_add_foreign) and I haven't been able to 
find how the page will be mapped read-only in the guest P2M. 
get_page_from_gfn will always return p2m_raw_rw for DOMID_XEN as it's a 
non translated domain.


Andrew and Jan, do you know how this is supposed to work when xentrace 
is used in a HVM domain? Does x86 Xen always mapped Read-Write the page?



+}
+else
+{
+*t = p2m_invalid;
+}


The brackets are not necessary for a single statement.


+    ret = paddr;
+}
+
  return ret;
  }




Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v3 3/5] xentrace: Timestamp support for ARM platform

2016-04-08 Thread Julien Grall

Hello Benjamin,

On 04/04/16 19:48, Benjamin Sanda wrote:

Moved get_cycles() to time.c and modified to return the core timestamp
tick count for use by the trace buffer timestamping routines in
xentrace. get_cycles() was moved to the C file to avoid including the
register specific header file in time.h and to commonize it with the
get_s_time() function. Also defined cycles_t as uint64_t to simplify
casting.


I'm not sure what you mean by "simplify casting".

The type cycles_t is not correctly defined for ARM32 because "unsigned 
long" is always 32-bits. However, the physical count register (CNTPCT) 
is always 64-bits. So the number of cycles would have been truncated.


The rest of the patch looks good to me.


get_s_time() was also modified to now use the updated get_cycles() to
retrieve the tick count instead of directly reading it.

Signed-off-by: Benjamin Sanda 

---
Changed since v2:
   * Combined v2 patches 7 and 6 into one patch in v3. No code change.

---
Changed since v1:
   * Moved get_cycles() to time.c
   * Added function prototype for get_cycles()
---
  xen/arch/arm/time.c|  9 -
  xen/include/asm-arm/time.h | 11 +--
  2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
index 7dae28b..9aface3 100644
--- a/xen/arch/arm/time.c
+++ b/xen/arch/arm/time.c
@@ -192,10 +192,17 @@ int __init init_xen_time(void)
  /* Return number of nanoseconds since boot */
  s_time_t get_s_time(void)
  {
-uint64_t ticks = READ_SYSREG64(CNTPCT_EL0) - boot_count;
+cycles_t ticks = get_cycles();
  return ticks_to_ns(ticks);
  }

+/* Return the number of ticks since boot */
+cycles_t get_cycles(void)
+{
+/* return raw tick count of main timer */
+return READ_SYSREG64(CNTPCT_EL0) - boot_count;
+}
+
  /* Set the timer to wake us up at a particular time.
   * Timeout is a Xen system time (nanoseconds since boot); 0 disables the 
timer.
   * Returns 1 on success; 0 if the timeout is too soon or is in the past. */
diff --git a/xen/include/asm-arm/time.h b/xen/include/asm-arm/time.h
index 5b9a31d..b57f4c1 100644
--- a/xen/include/asm-arm/time.h
+++ b/xen/include/asm-arm/time.h
@@ -5,12 +5,8 @@
  DT_MATCH_COMPATIBLE("arm,armv7-timer"), \
  DT_MATCH_COMPATIBLE("arm,armv8-timer")

-typedef unsigned long cycles_t;
-
-static inline cycles_t get_cycles (void)
-{
-return 0;
-}
+/* Tick count type */
+typedef uint64_t cycles_t;

  /* List of timer's IRQ */
  enum timer_ppi
@@ -37,6 +33,9 @@ extern void init_timer_interrupt(void);
  /* Counter value at boot time */
  extern uint64_t boot_count;

+/* Get raw system tick count */
+cycles_t get_cycles(void);
+
  extern s_time_t ticks_to_ns(uint64_t ticks);
  extern uint64_t ns_to_ticks(s_time_t ns);




Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH v3 4/5] xentrace: Trace Buffer Initialization on ARM

2016-04-08 Thread Julien Grall

Hello Benjamin,

On 04/04/16 19:48, Benjamin Sanda wrote:

Added call to init_trace_bufs() to initialize the trace buffers for
use by xentrace.

Signed-off-by: Benjamin Sanda 


Acked-by: Julien Grall 

Regards,

--
Julien Grall

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


  1   2   3   4   5   6   7   8   9   10   >