Re: [Xen-devel] [PATCH 3/6] xentrace: P2M lookup suport for ARM platform
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
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
(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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
(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
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
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
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)
(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
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
(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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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?
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
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.
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
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
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
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
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
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
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
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)
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?
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.
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
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
(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
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
(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?
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
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
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
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
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
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
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
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
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
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()
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
(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
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
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