On Fri, Sep 14, 2012 at 12:13:04PM +0100, Stefano Stabellini wrote:

Use r12 to pass the hypercall number to the hypervisor.

We need a register to pass the hypercall number because we might not know it at compile time and HVC only takes an immediate argument.

Among the available registers r12 seems to be the best choice because it is defined as "intra-procedure call scratch register".

Use the ISS to pass an hypervisor specific tag.

Changes in v2:

• define an HYPERCALL macro for 5 arguments hypercall wrappers, even if

at the moment is unused;

• use ldm instead of pop;
• fix up comments.

Signed-off-by: Stefano Stabellini stefano.stabellini@eu.citrix.com

Reviewed-by: Konrad Rzeszutek Wilk konrad.wilk@oracle.com

.. since this is all in non-x86 and non-Xen generic parts so cannot provide you with an Ack :-)

---

arch/arm/include/asm/xen/hypercall.h | 50 ++++++++++++++++ arch/arm/xen/Makefile | 2 +- arch/arm/xen/hypercall.S | 106 ++++++++++++++++++++++++++++++++++ 3 files changed, 157 insertions(+), 1 deletions(-) create mode 100644 arch/arm/include/asm/xen/hypercall.h create mode 100644 arch/arm/xen/hypercall.S

diff --git a/arch/arm/include/asm/xen/hypercall.h b/arch/arm/include/asm/xen/hypercall.h new file mode 100644 index 0000000..4ac0624 --- /dev/null +++ b/arch/arm/include/asm/xen/hypercall.h @@ -0,0 +1,50 @@ +/******************************************************************************

• • hypercall.h
• • Linux-specific hypervisor handling.
• • Stefano Stabellini stefano.stabellini@eu.citrix.com, Citrix, 2012
• • This program is free software; you can redistribute it and/or
• • modify it under the terms of the GNU General Public License version 2
• • as published by the Free Software Foundation; or, when distributed
• • separately from the Linux kernel or incorporated into other
• • software packages, subject to the following license:
• • Permission is hereby granted, free of charge, to any person obtaining a copy
• • of this source file (the "Software"), to deal in the Software without
• • restriction, including without limitation the rights to use, copy, modify,
• • merge, publish, distribute, sublicense, and/or sell copies of the Software,
• • and to permit persons to whom the Software is furnished to do so, subject to
• • the following conditions:
• • The above copyright notice and this permission notice shall be included in
• • all copies or substantial portions of the Software.
• • THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
• • IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
• • FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
• • AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
• • LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
• • FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
• • IN THE SOFTWARE.
• */

+#ifndef _ASM_ARM_XEN_HYPERCALL_H +#define _ASM_ARM_XEN_HYPERCALL_H

+#include <xen/interface/xen.h>

+long privcmd_call(unsigned call, unsigned long a1,

• unsigned long a2, unsigned long a3,
  

• unsigned long a4, unsigned long a5);
  

+int HYPERVISOR_xen_version(int cmd, void *arg); +int HYPERVISOR_console_io(int cmd, int count, char *str); +int HYPERVISOR_grant_table_op(unsigned int cmd, void *uop, unsigned int count); +int HYPERVISOR_sched_op(int cmd, void *arg); +int HYPERVISOR_event_channel_op(int cmd, void *arg); +unsigned long HYPERVISOR_hvm_op(int op, void *arg); +int HYPERVISOR_memory_op(unsigned int cmd, void *arg); +int HYPERVISOR_physdev_op(int cmd, void *arg);

+#endif /* _ASM_ARM_XEN_HYPERCALL_H */ diff --git a/arch/arm/xen/Makefile b/arch/arm/xen/Makefile index 0bad594..b9d6acc 100644 --- a/arch/arm/xen/Makefile +++ b/arch/arm/xen/Makefile @@ -1 +1 @@ -obj-y := enlighten.o +obj-y := enlighten.o hypercall.o diff --git a/arch/arm/xen/hypercall.S b/arch/arm/xen/hypercall.S new file mode 100644 index 0000000..074f5ed --- /dev/null +++ b/arch/arm/xen/hypercall.S @@ -0,0 +1,106 @@ +/******************************************************************************

• • hypercall.S
• • Xen hypercall wrappers
• • Stefano Stabellini stefano.stabellini@eu.citrix.com, Citrix, 2012
• • This program is free software; you can redistribute it and/or
• • modify it under the terms of the GNU General Public License version 2
• • as published by the Free Software Foundation; or, when distributed
• • separately from the Linux kernel or incorporated into other
• • software packages, subject to the following license:
• • Permission is hereby granted, free of charge, to any person obtaining a copy
• • of this source file (the "Software"), to deal in the Software without
• • restriction, including without limitation the rights to use, copy, modify,
• • merge, publish, distribute, sublicense, and/or sell copies of the Software,
• • and to permit persons to whom the Software is furnished to do so, subject to
• • the following conditions:
• • The above copyright notice and this permission notice shall be included in
• • all copies or substantial portions of the Software.
• • THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
• • IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
• • FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
• • AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
• • LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
• • FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
• • IN THE SOFTWARE.
• */

+/*

• • The Xen hypercall calling convention is very similar to the ARM
• • procedure calling convention: the first paramter is passed in r0, the
• • second in r1, the third in r2 and the fourth in r3. Considering that
• • Xen hypercalls have 5 arguments at most, the fifth paramter is passed
• • in r4, differently from the procedure calling convention of using the
• • stack for that case.
• • The hypercall number is passed in r12.
• • The return value is in r0.
• • The hvc ISS is required to be 0xEA1, that is the Xen specific ARM
• • hypercall tag.
• */

+#include <linux/linkage.h> +#include <asm/assembler.h> +#include <xen/interface/xen.h>

+/* HVC 0xEA1 */ +#ifdef CONFIG_THUMB2_KERNEL +#define xen_hvc .word 0xf7e08ea1 +#else +#define xen_hvc .word 0xe140ea71 +#endif

+#define HYPERCALL_SIMPLE(hypercall) \ +ENTRY(HYPERVISOR_##hypercall) \

• mov r12, #__HYPERVISOR_##hypercall; \
• xen_hvc; \
• mov pc, lr; \

+ENDPROC(HYPERVISOR_##hypercall)

+#define HYPERCALL0 HYPERCALL_SIMPLE +#define HYPERCALL1 HYPERCALL_SIMPLE +#define HYPERCALL2 HYPERCALL_SIMPLE +#define HYPERCALL3 HYPERCALL_SIMPLE +#define HYPERCALL4 HYPERCALL_SIMPLE

+#define HYPERCALL5(hypercall) \ +ENTRY(HYPERVISOR_##hypercall) \

• stmdb sp!, {r4} \
• ldr r4, [sp, #4] \
• mov r12, #__HYPERVISOR_##hypercall; \
• xen_hvc \
• ldm sp!, {r4} \
• mov pc, lr \

+ENDPROC(HYPERVISOR_##hypercall)

•            .text
  

+HYPERCALL2(xen_version); +HYPERCALL3(console_io); +HYPERCALL3(grant_table_op); +HYPERCALL2(sched_op); +HYPERCALL2(event_channel_op); +HYPERCALL2(hvm_op); +HYPERCALL2(memory_op); +HYPERCALL2(physdev_op);

+ENTRY(privcmd_call)

• stmdb sp!, {r4}
• mov r12, r0
• mov r0, r1
• mov r1, r2
• mov r2, r3
• ldr r3, [sp, #8]
• ldr r4, [sp, #4]
• xen_hvc
• ldm sp!, {r4}
• mov pc, lr

+ENDPROC(privcmd_call);

1.7.2.5

On Fri, 14 Sep 2012, Marc Zyngier wrote:

On 14/09/12 12:13, Stefano Stabellini wrote:

...

Use r12 to pass the hypercall number to the hypervisor.

We need a register to pass the hypercall number because we might not know it at compile time and HVC only takes an immediate argument.

Among the available registers r12 seems to be the best choice because it is defined as "intra-procedure call scratch register".

Use the ISS to pass an hypervisor specific tag.

Changes in v2:

• define an HYPERCALL macro for 5 arguments hypercall wrappers, even if

at the moment is unused;

• use ldm instead of pop;
• fix up comments.

Signed-off-by: Stefano Stabellini stefano.stabellini@eu.citrix.com

arch/arm/include/asm/xen/hypercall.h | 50 ++++++++++++++++ arch/arm/xen/Makefile | 2 +- arch/arm/xen/hypercall.S | 106 ++++++++++++++++++++++++++++++++++ 3 files changed, 157 insertions(+), 1 deletions(-) create mode 100644 arch/arm/include/asm/xen/hypercall.h create mode 100644 arch/arm/xen/hypercall.S

diff --git a/arch/arm/include/asm/xen/hypercall.h b/arch/arm/include/asm/xen/hypercall.h new file mode 100644 index 0000000..4ac0624 --- /dev/null +++ b/arch/arm/include/asm/xen/hypercall.h @@ -0,0 +1,50 @@ +/******************************************************************************

• • hypercall.h
• • Linux-specific hypervisor handling.
• • Stefano Stabellini stefano.stabellini@eu.citrix.com, Citrix, 2012
• • This program is free software; you can redistribute it and/or
• • modify it under the terms of the GNU General Public License version 2
• • as published by the Free Software Foundation; or, when distributed
• • separately from the Linux kernel or incorporated into other
• • software packages, subject to the following license:
• • Permission is hereby granted, free of charge, to any person obtaining a copy
• • of this source file (the "Software"), to deal in the Software without
• • restriction, including without limitation the rights to use, copy, modify,
• • merge, publish, distribute, sublicense, and/or sell copies of the Software,
• • and to permit persons to whom the Software is furnished to do so, subject to
• • the following conditions:
• • The above copyright notice and this permission notice shall be included in
• • all copies or substantial portions of the Software.
• • THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
• • IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
• • FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
• • AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
• • LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
• • FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
• • IN THE SOFTWARE.
• */

+#ifndef _ASM_ARM_XEN_HYPERCALL_H +#define _ASM_ARM_XEN_HYPERCALL_H

+#include <xen/interface/xen.h>

+long privcmd_call(unsigned call, unsigned long a1,

• unsigned long a2, unsigned long a3,
  

• unsigned long a4, unsigned long a5);
  

+int HYPERVISOR_xen_version(int cmd, void *arg); +int HYPERVISOR_console_io(int cmd, int count, char *str); +int HYPERVISOR_grant_table_op(unsigned int cmd, void *uop, unsigned int count); +int HYPERVISOR_sched_op(int cmd, void *arg); +int HYPERVISOR_event_channel_op(int cmd, void *arg); +unsigned long HYPERVISOR_hvm_op(int op, void *arg); +int HYPERVISOR_memory_op(unsigned int cmd, void *arg); +int HYPERVISOR_physdev_op(int cmd, void *arg);

+#endif /* _ASM_ARM_XEN_HYPERCALL_H */ diff --git a/arch/arm/xen/Makefile b/arch/arm/xen/Makefile index 0bad594..b9d6acc 100644 --- a/arch/arm/xen/Makefile +++ b/arch/arm/xen/Makefile @@ -1 +1 @@ -obj-y := enlighten.o +obj-y := enlighten.o hypercall.o diff --git a/arch/arm/xen/hypercall.S b/arch/arm/xen/hypercall.S new file mode 100644 index 0000000..074f5ed --- /dev/null +++ b/arch/arm/xen/hypercall.S @@ -0,0 +1,106 @@ +/******************************************************************************

• • hypercall.S
• • Xen hypercall wrappers
• • Stefano Stabellini stefano.stabellini@eu.citrix.com, Citrix, 2012
• • This program is free software; you can redistribute it and/or
• • modify it under the terms of the GNU General Public License version 2
• • as published by the Free Software Foundation; or, when distributed
• • separately from the Linux kernel or incorporated into other
• • software packages, subject to the following license:
• • Permission is hereby granted, free of charge, to any person obtaining a copy
• • of this source file (the "Software"), to deal in the Software without
• • restriction, including without limitation the rights to use, copy, modify,
• • merge, publish, distribute, sublicense, and/or sell copies of the Software,
• • and to permit persons to whom the Software is furnished to do so, subject to
• • the following conditions:
• • The above copyright notice and this permission notice shall be included in
• • all copies or substantial portions of the Software.
• • THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
• • IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
• • FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
• • AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
• • LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
• • FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
• • IN THE SOFTWARE.
• */

+/*

• • The Xen hypercall calling convention is very similar to the ARM
• • procedure calling convention: the first paramter is passed in r0, the
• • second in r1, the third in r2 and the fourth in r3. Considering that
• • Xen hypercalls have 5 arguments at most, the fifth paramter is passed
• • in r4, differently from the procedure calling convention of using the
• • stack for that case.
• • The hypercall number is passed in r12.
• • The return value is in r0.
• • The hvc ISS is required to be 0xEA1, that is the Xen specific ARM
• • hypercall tag.
• */

+#include <linux/linkage.h> +#include <asm/assembler.h> +#include <xen/interface/xen.h>

+/* HVC 0xEA1 */ +#ifdef CONFIG_THUMB2_KERNEL +#define xen_hvc .word 0xf7e08ea1 +#else +#define xen_hvc .word 0xe140ea71 +#endif

You should consider using Dave Martin's opcode injection series for that. The patches are already in Russell's for-next branch.

I am all for using Dave's patches and I have even already sent a patch to change the wrappers to use __HVC:

http://marc.info/?l=linux-kernel&m=134513261623427&w=2

I have only temporarely dropped it, only to reduce the amount of external dependencies of this series. It already depends on Konrad's stable/for-linus-3.7, so I would rather send this patch out later on its own...

On Fri, Sep 14, 2012 at 12:13:08PM +0100, Stefano Stabellini wrote:

Add a doc to describe the Xen ARM device tree bindings

Changes in v4:

• "xen,xen" should be last as it is less specific;
• update reg property using 2 address-cells and 2 size-cells.

Signed-off-by: Stefano Stabellini stefano.stabellini@eu.citrix.com CC: devicetree-discuss@lists.ozlabs.org CC: David Vrabel david.vrabel@citrix.com CC: Rob Herring robherring2@gmail.com CC: Dave Martin dave.martin@linaro.org

---

Documentation/devicetree/bindings/arm/xen.txt | 22 ++++++++++++++++++++++ 1 files changed, 22 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/arm/xen.txt

diff --git a/Documentation/devicetree/bindings/arm/xen.txt b/Documentation/devicetree/bindings/arm/xen.txt new file mode 100644 index 0000000..1f8f7d4 --- /dev/null +++ b/Documentation/devicetree/bindings/arm/xen.txt @@ -0,0 +1,22 @@ +* Xen hypervisor device tree bindings

+Xen ARM virtual platforms shall have the following properties:

+- compatible:

• compatible = "xen,xen-<version>", "xen,xen";
• where <version> is the version of the Xen ABI of the platform.

+- reg: specifies the base physical address and size of a region in

• memory where the grant table should be mapped to, using an
• HYPERVISOR_memory_op hypercall.

+- interrupts: the interrupt used by Xen to inject event notifications.

Its singular here.. but in the example its plurar. What if you use multiple of the same number ("16 0xf")?

+Example:

+hypervisor {

• compatible = "xen,xen-4.3", "xen,xen";
• reg = <0 0xb0000000 0 0x20000>;

So two grant tables?

Hm, physical address is zero, and the size is 0xbignumber? Or is the '0' denotating a seperator of arguments, so it is 0xb000.. for physical address and 0x20000 for size?

• interrupts = <1 15 0xf08>;

+};

1.7.2.5

On 09/14/2012 09:26 AM, Stefano Stabellini wrote:

On Fri, 14 Sep 2012, Konrad Rzeszutek Wilk wrote:

...

On Fri, Sep 14, 2012 at 12:13:08PM +0100, Stefano Stabellini wrote:

...

Add a doc to describe the Xen ARM device tree bindings

Changes in v4:

• "xen,xen" should be last as it is less specific;
• update reg property using 2 address-cells and 2 size-cells.

Signed-off-by: Stefano Stabellini stefano.stabellini@eu.citrix.com CC: devicetree-discuss@lists.ozlabs.org CC: David Vrabel david.vrabel@citrix.com CC: Rob Herring robherring2@gmail.com CC: Dave Martin dave.martin@linaro.org

---

Documentation/devicetree/bindings/arm/xen.txt | 22 ++++++++++++++++++++++ 1 files changed, 22 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/arm/xen.txt

diff --git a/Documentation/devicetree/bindings/arm/xen.txt b/Documentation/devicetree/bindings/arm/xen.txt new file mode 100644 index 0000000..1f8f7d4 --- /dev/null +++ b/Documentation/devicetree/bindings/arm/xen.txt @@ -0,0 +1,22 @@ +* Xen hypervisor device tree bindings

+Xen ARM virtual platforms shall have the following properties:

State that they are part of top-level "hypervisor" node.

...

...

+- compatible:

• compatible = "xen,xen-<version>", "xen,xen";
• where <version> is the version of the Xen ABI of the platform.

+- reg: specifies the base physical address and size of a region in

• memory where the grant table should be mapped to, using an
• HYPERVISOR_memory_op hypercall.

+- interrupts: the interrupt used by Xen to inject event notifications.

Its singular here.. but in the example its plurar. What if you use multiple of the same number ("16 0xf")?

The "interrupts" property in the example below is a standard property to describe interrupts. We just happen to declare only one interrupt.

From the device tree point of view it would be possible to declare more than one interrupt here, but Xen only supports one really.

Regarding the three cells used in the example (<1 15 0xf08>), they have a specific meaning in the GIC context:

""" The 1st cell is the interrupt type; 0 for SPI interrupts, 1 for PPI interrupts.

The 2nd cell contains the interrupt number for the interrupt type. SPI interrupts are in the range [0-987]. PPI interrupts are in the range [0-15].

The 3rd cell is the flags, encoded as follows: bits[3:0] trigger type and level flags. 1 = low-to-high edge triggered 2 = high-to-low edge triggered 4 = active high level-sensitive 8 = active low level-sensitive bits[15:8] PPI interrupt cpu mask. Each bit corresponds to each of the 8 possible cpus attached to the GIC. A bit set to '1' indicated the interrupt is wired to that CPU. Only valid for PPI interrupts. """

So <1 15 0xf08> means the last PPI.

Since it is a PPI, it is handled differently than a normal interrupt. That is fine, but you should somehow state that a GIC node is also required.

...

...

+Example:

+hypervisor {

• compatible = "xen,xen-4.3", "xen,xen";
• reg = <0 0xb0000000 0 0x20000>;

So two grant tables?

Hm, physical address is zero, and the size is 0xbignumber? Or is the '0' denotating a seperator of arguments, so it is 0xb000.. for physical address and 0x20000 for size?

from http://devicetree.org/Device_Tree_Usage:

"Each addressable device gets a reg which is a list of tuples in the form reg = <address1 length1 [address2 length2] [address3 length3] ... Each tuple represents an address range used by the device. Each address value is a list of one or more 32 bit integers called cells. Similarly, the length value can either be a list of cells, or empty."

In this case the address is: [0 0xb0000000], that means 0x00000000b0000000, and the length is [0 0x20000], that means 0x0000000000020000.

But the size depends on #size-cells and #address-cells. I would expect those to be 1 for a 32-bit guest.

Rob

On Mon, Sep 17, 2012 at 03:12:11PM +0100, Stefano Stabellini wrote:

On Mon, 17 Sep 2012, Rob Herring wrote:

...

On 09/14/2012 09:26 AM, Stefano Stabellini wrote:

...

On Fri, 14 Sep 2012, Konrad Rzeszutek Wilk wrote:

...

On Fri, Sep 14, 2012 at 12:13:08PM +0100, Stefano Stabellini wrote:

...

Add a doc to describe the Xen ARM device tree bindings

Changes in v4:

• "xen,xen" should be last as it is less specific;
• update reg property using 2 address-cells and 2 size-cells.

Signed-off-by: Stefano Stabellini stefano.stabellini@eu.citrix.com CC: devicetree-discuss@lists.ozlabs.org CC: David Vrabel david.vrabel@citrix.com CC: Rob Herring robherring2@gmail.com CC: Dave Martin dave.martin@linaro.org

---

Documentation/devicetree/bindings/arm/xen.txt | 22 ++++++++++++++++++++++ 1 files changed, 22 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/arm/xen.txt

diff --git a/Documentation/devicetree/bindings/arm/xen.txt b/Documentation/devicetree/bindings/arm/xen.txt new file mode 100644 index 0000000..1f8f7d4 --- /dev/null +++ b/Documentation/devicetree/bindings/arm/xen.txt @@ -0,0 +1,22 @@ +* Xen hypervisor device tree bindings

+Xen ARM virtual platforms shall have the following properties:

State that they are part of top-level "hypervisor" node.

OK

...

...

...

...

+- compatible:

• compatible = "xen,xen-<version>", "xen,xen";
• where <version> is the version of the Xen ABI of the platform.

+- reg: specifies the base physical address and size of a region in

• memory where the grant table should be mapped to, using an
• HYPERVISOR_memory_op hypercall.

+- interrupts: the interrupt used by Xen to inject event notifications.

Its singular here.. but in the example its plurar. What if you use multiple of the same number ("16 0xf")?

The "interrupts" property in the example below is a standard property to describe interrupts. We just happen to declare only one interrupt.

From the device tree point of view it would be possible to declare more than one interrupt here, but Xen only supports one really.

Regarding the three cells used in the example (<1 15 0xf08>), they have a specific meaning in the GIC context:

""" The 1st cell is the interrupt type; 0 for SPI interrupts, 1 for PPI interrupts.

The 2nd cell contains the interrupt number for the interrupt type. SPI interrupts are in the range [0-987]. PPI interrupts are in the range [0-15].

The 3rd cell is the flags, encoded as follows: bits[3:0] trigger type and level flags. 1 = low-to-high edge triggered 2 = high-to-low edge triggered 4 = active high level-sensitive 8 = active low level-sensitive bits[15:8] PPI interrupt cpu mask. Each bit corresponds to each of the 8 possible cpus attached to the GIC. A bit set to '1' indicated the interrupt is wired to that CPU. Only valid for PPI interrupts. """

So <1 15 0xf08> means the last PPI.

Since it is a PPI, it is handled differently than a normal interrupt. That is fine, but you should somehow state that a GIC node is also required.

Yes, good idea

...

...

...

...

+Example:

+hypervisor {

• compatible = "xen,xen-4.3", "xen,xen";
• reg = <0 0xb0000000 0 0x20000>;

So two grant tables?

Hm, physical address is zero, and the size is 0xbignumber? Or is the '0' denotating a seperator of arguments, so it is 0xb000.. for physical address and 0x20000 for size?

from http://devicetree.org/Device_Tree_Usage:

"Each addressable device gets a reg which is a list of tuples in the form reg = <address1 length1 [address2 length2] [address3 length3] ... Each tuple represents an address range used by the device. Each address value is a list of one or more 32 bit integers called cells. Similarly, the length value can either be a list of cells, or empty."

In this case the address is: [0 0xb0000000], that means 0x00000000b0000000, and the length is [0 0x20000], that means 0x0000000000020000.

But the size depends on #size-cells and #address-cells. I would expect those to be 1 for a 32-bit guest.

I was looking at the Versatile Express DTS (vexpress-v2p-ca15-tc1.dts) that on Linux v3.6-rc5 has:

#address-cells = <2>; #size-cells = <2>;

Some core tiles on vexpress use physical addresses beyond 4G. But many 32-bit platforms (including some supporting the virtualization extensions) may not. There's no reason for such platforms to set these properties to <2>.

What should I use for the example in this doc?

Looking at other files in Documentation/device-tree/bindings/, it looks like the common example configuration is for #address-cells and #size-cells to be 1.

So, assuming that those are 1 is probably best for examples.

You could state this explicitly for good measure, but the need to expand reg properties (and other related properties) to match the parent bus #address-cells and #size-cells is a standard device-tree concept, so I think it doesn't make sense to describe the implications in detail on a per-binding basis.

Cheers ---Dave

On Fri, Sep 14, 2012 at 12:13:09PM +0100, Stefano Stabellini wrote:

Check for a node in the device tree compatible with "xen,xen", if it is present set xen_domain_type to XEN_HVM_DOMAIN and continue initialization.

Map the real shared info page using XENMEM_add_to_physmap with XENMAPSPACE_shared_info.

Changes in v4:

• simpler parsing of Xen version in the compatible DT node.

Changes in v3:

• use the "xen,xen" notation rather than "arm,xen";
• add an additional check on the presence of the Xen version.

Changes in v2:

• replace pr_info with pr_debug.

Signed-off-by: Stefano Stabellini stefano.stabellini@eu.citrix.com

arch/arm/xen/enlighten.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 61 insertions(+), 0 deletions(-)

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c index c535540..6a0217d 100644 --- a/arch/arm/xen/enlighten.c +++ b/arch/arm/xen/enlighten.c @@ -5,6 +5,9 @@ #include <asm/xen/hypervisor.h> #include <asm/xen/hypercall.h> #include <linux/module.h> +#include <linux/of.h> +#include <linux/of_irq.h> +#include <linux/of_address.h> struct start_info _xen_start_info; struct start_info *xen_start_info = &_xen_start_info; @@ -33,3 +36,61 @@ int xen_remap_domain_mfn_range(struct vm_area_struct *vma, return -ENOSYS; } EXPORT_SYMBOL_GPL(xen_remap_domain_mfn_range);

+/*

• • see Documentation/devicetree/bindings/arm/xen.txt for the
• • documentation of the Xen Device Tree format.
• */

+static int __init xen_guest_init(void) +{

• struct xen_add_to_physmap xatp;
• static struct shared_info *shared_info_page = 0;
• struct device_node *node;
• int len;
• const char *s = NULL;
• const char *version = NULL;
• const char *xen_prefix = "xen,xen-";
• node = of_find_compatible_node(NULL, NULL, "xen,xen");
• if (!node) {

• pr_debug("No Xen support\n");
  

• return 0;
  

• }
• s = of_get_property(node, "compatible", &len);
• if (strlen(xen_prefix) + 3 < len &&

• 	!strncmp(xen_prefix, s, strlen(xen_prefix)))
  

If we have version '4.3.1' won't this trip us over?

Or if we only have 'major' and 'minor', then won't '4.11' trip us over too?

• version = s + strlen(xen_prefix);
  

• if (version == NULL) {

• pr_debug("Xen version not found\n");
  

• return 0;
  

• }
• xen_domain_type = XEN_HVM_DOMAIN;
• if (!shared_info_page)

• shared_info_page = (struct shared_info *)
  

• 	get_zeroed_page(GFP_KERNEL);
  

• if (!shared_info_page) {

• pr_err("not enough memory\n");
  

• return -ENOMEM;
  

• }
• xatp.domid = DOMID_SELF;
• xatp.idx = 0;
• xatp.space = XENMAPSPACE_shared_info;
• xatp.gpfn = __pa(shared_info_page) >> PAGE_SHIFT;
• if (HYPERVISOR_memory_op(XENMEM_add_to_physmap, &xatp))

• BUG();
  

• HYPERVISOR_shared_info = (struct shared_info *)shared_info_page;
• /* xen_vcpu is a pointer to the vcpu_info struct in the shared_info

• * page, we use it in the event channel upcall and in some pvclock
  

• * related functions. We don't need the vcpu_info placement
  

• * optimizations because we don't use any pv_mmu or pv_irq op on
  

• * HVM.
  

• * The shared info contains exactly 1 CPU (the boot CPU). The guest
  

• * is required to use VCPUOP_register_vcpu_info to place vcpu info
  

• * for secondary CPUs as they are brought up. */
  

• per_cpu(xen_vcpu, 0) = &HYPERVISOR_shared_info->vcpu_info[0];
• return 0;

+}

+core_initcall(xen_guest_init);

1.7.2.5

On Fri, Sep 14, 2012 at 12:13:11PM +0100, Stefano Stabellini wrote:

All the original Xen headers have xen_ulong_t as unsigned long type, however when they have been imported in Linux, xen_ulong_t has been replaced with unsigned long. That might work for x86 and ia64 but it does not for arm. Bring back xen_ulong_t and let each architecture define xen_ulong_t as they see fit.

Also explicitly size pointers (__DEFINE_GUEST_HANDLE) to 64 bit.

Changes in v3:

• remove the incorrect changes to multicall_entry;
• remove the change to apic_physbase.

Signed-off-by: Stefano Stabellini stefano.stabellini@eu.citrix.com

Acked-by: Konrad Rzeszutek Wilk konrad.wilk@oracle.com for the generic parts; all other: Reviewed-by: Konrad Rzeszutek Wilk konrad.wilk@oracle.com

---

arch/arm/include/asm/xen/interface.h | 8 ++++++-- arch/ia64/include/asm/xen/interface.h | 1 + arch/x86/include/asm/xen/interface.h | 1 + include/xen/interface/memory.h | 12 ++++++------ include/xen/interface/physdev.h | 2 +- include/xen/interface/version.h | 2 +- 6 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/arch/arm/include/asm/xen/interface.h b/arch/arm/include/asm/xen/interface.h index 74c72b5..ae05e56 100644 --- a/arch/arm/include/asm/xen/interface.h +++ b/arch/arm/include/asm/xen/interface.h @@ -9,8 +9,11 @@ #include <linux/types.h> +#define uint64_aligned_t uint64_t __attribute__((aligned(8)))

#define __DEFINE_GUEST_HANDLE(name, type) \

• typedef type * __guest_handle_ ## name

• typedef struct { union { type *p; uint64_aligned_t q; }; } \

•    __guest_handle_ ## name
  

#define DEFINE_GUEST_HANDLE_STRUCT(name) \ __DEFINE_GUEST_HANDLE(name, struct name) @@ -21,13 +24,14 @@ do { \ if (sizeof(hnd) == 8) \ *(uint64_t *)&(hnd) = 0; \

• (hnd) = val;				\
  

• (hnd).p = val;				\
  

  } while (0)

#ifndef __ASSEMBLY__ /* Explicitly size integers that represent pfns in the interface with

• Xen so that we can have one ABI that works for 32 and 64 bit guests. */

typedef uint64_t xen_pfn_t; +typedef uint64_t xen_ulong_t; /* Guest handles for primitive C types. */ __DEFINE_GUEST_HANDLE(uchar, unsigned char); __DEFINE_GUEST_HANDLE(uint, unsigned int); diff --git a/arch/ia64/include/asm/xen/interface.h b/arch/ia64/include/asm/xen/interface.h index 686464e..7c83445 100644 --- a/arch/ia64/include/asm/xen/interface.h +++ b/arch/ia64/include/asm/xen/interface.h @@ -71,6 +71,7 @@

• with Xen so that we could have one ABI that works for 32 and 64 bit
• guests. */

typedef unsigned long xen_pfn_t; +typedef unsigned long xen_ulong_t; /* Guest handles for primitive C types. */ __DEFINE_GUEST_HANDLE(uchar, unsigned char); __DEFINE_GUEST_HANDLE(uint, unsigned int); diff --git a/arch/x86/include/asm/xen/interface.h b/arch/x86/include/asm/xen/interface.h index 2289075..25cc8df 100644 --- a/arch/x86/include/asm/xen/interface.h +++ b/arch/x86/include/asm/xen/interface.h @@ -51,6 +51,7 @@

• with Xen so that on ARM we can have one ABI that works for 32 and 64
• bit guests. */

typedef unsigned long xen_pfn_t; +typedef unsigned long xen_ulong_t; /* Guest handles for primitive C types. */ __DEFINE_GUEST_HANDLE(uchar, unsigned char); __DEFINE_GUEST_HANDLE(uint, unsigned int); diff --git a/include/xen/interface/memory.h b/include/xen/interface/memory.h index abbbff0..b5c3098 100644 --- a/include/xen/interface/memory.h +++ b/include/xen/interface/memory.h @@ -34,7 +34,7 @@ struct xen_memory_reservation { GUEST_HANDLE(xen_pfn_t) extent_start; /* Number of extents, and size/alignment of each (2^extent_order pages). */

• unsigned long nr_extents;

• xen_ulong_t nr_extents; unsigned int extent_order;

/* @@ -92,7 +92,7 @@ struct xen_memory_exchange { * command will be non-zero. * 5. THIS FIELD MUST BE INITIALISED TO ZERO BY THE CALLER! */

• unsigned long nr_exchanged;

• xen_ulong_t nr_exchanged;

}; DEFINE_GUEST_HANDLE_STRUCT(xen_memory_exchange); @@ -148,8 +148,8 @@ DEFINE_GUEST_HANDLE_STRUCT(xen_machphys_mfn_list); */ #define XENMEM_machphys_mapping 12 struct xen_machphys_mapping {

• unsigned long v_start, v_end; /* Start and end virtual addresses. */
• unsigned long max_mfn; /* Maximum MFN that can be looked up. */

• xen_ulong_t v_start, v_end; /* Start and end virtual addresses. */
• xen_ulong_t max_mfn; /* Maximum MFN that can be looked up. */

}; DEFINE_GUEST_HANDLE_STRUCT(xen_machphys_mapping_t); @@ -169,7 +169,7 @@ struct xen_add_to_physmap { unsigned int space; /* Index into source mapping space. */

• unsigned long idx;

• xen_ulong_t idx;

/* GPFN where the source mapping page should appear. */ xen_pfn_t gpfn; @@ -186,7 +186,7 @@ struct xen_translate_gpfn_list { domid_t domid; /* Length of list. */

• unsigned long nr_gpfns;

• xen_ulong_t nr_gpfns;

/* List of GPFNs to translate. */ GUEST_HANDLE(ulong) gpfn_list; diff --git a/include/xen/interface/physdev.h b/include/xen/interface/physdev.h index 9ce788d..f616514 100644 --- a/include/xen/interface/physdev.h +++ b/include/xen/interface/physdev.h @@ -56,7 +56,7 @@ struct physdev_eoi { #define PHYSDEVOP_pirq_eoi_gmfn_v2 28 struct physdev_pirq_eoi_gmfn { /* IN */

• unsigned long gmfn;

• xen_ulong_t gmfn;

}; /* diff --git a/include/xen/interface/version.h b/include/xen/interface/version.h index e8b6519..30280c9 100644 --- a/include/xen/interface/version.h +++ b/include/xen/interface/version.h @@ -45,7 +45,7 @@ struct xen_changeset_info { #define XENVER_platform_parameters 5 struct xen_platform_parameters {

• unsigned long virt_start;

• xen_ulong_t virt_start;

};

#define XENVER_get_features 6

1.7.2.5

On Fri, Sep 14, 2012 at 12:13:12PM +0100, Stefano Stabellini wrote:

bind_evtchn_to_irqhandler can legitimately return 0 (irq 0): it is not an error.

If Linux is running as an HVM domain and is running as Dom0, use xenstored_local_init to initialize the xenstore page and event channel.

Let me stick it in my tree and see how it works overnight with HVM and PV guests.

Changes in v4:

• do not xs_reset_watches on dom0.

Changes in v2:

• refactor xenbus_init.

Signed-off-by: Stefano Stabellini stefano.stabellini@eu.citrix.com

drivers/xen/xenbus/xenbus_comms.c | 2 +- drivers/xen/xenbus/xenbus_probe.c | 62 +++++++++++++++++++++++++----------- drivers/xen/xenbus/xenbus_xs.c | 3 +- 3 files changed, 46 insertions(+), 21 deletions(-)

diff --git a/drivers/xen/xenbus/xenbus_comms.c b/drivers/xen/xenbus/xenbus_comms.c index 52fe7ad..c5aa55c 100644 --- a/drivers/xen/xenbus/xenbus_comms.c +++ b/drivers/xen/xenbus/xenbus_comms.c @@ -224,7 +224,7 @@ int xb_init_comms(void) int err; err = bind_evtchn_to_irqhandler(xen_store_evtchn, wake_waiting, 0, "xenbus", &xb_waitq);

• if (err <= 0) {
  

• if (err < 0) {
  printk(KERN_ERR "XENBUS request irq failed %i\n", err);
  return err;
  

  }

diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c index b793723..a67ccc0 100644 --- a/drivers/xen/xenbus/xenbus_probe.c +++ b/drivers/xen/xenbus/xenbus_probe.c @@ -719,37 +719,61 @@ static int __init xenstored_local_init(void) return err; } +enum xenstore_init {

• UNKNOWN,
• PV,
• HVM,
• LOCAL,

+}; static int __init xenbus_init(void) { int err = 0;

• enum xenstore_init usage = UNKNOWN;
• uint64_t v = 0;

if (!xen_domain()) return -ENODEV; xenbus_ring_ops_init();

• if (xen_hvm_domain()) {

• uint64_t v = 0;
  

• err = hvm_get_parameter(HVM_PARAM_STORE_EVTCHN, &v);
  

• if (err)
  

• 	goto out_error;
  

• xen_store_evtchn = (int)v;
  

• err = hvm_get_parameter(HVM_PARAM_STORE_PFN, &v);
  

• if (err)
  

• 	goto out_error;
  

• xen_store_mfn = (unsigned long)v;
  

• xen_store_interface = ioremap(xen_store_mfn << PAGE_SHIFT, PAGE_SIZE);
  

• } else {

• xen_store_evtchn = xen_start_info->store_evtchn;
  

• xen_store_mfn = xen_start_info->store_mfn;
  

• if (xen_store_evtchn)
  

• 	xenstored_ready = 1;
  

• else {
  

• if (xen_pv_domain())

• usage = PV;
  

• if (xen_hvm_domain())

• usage = HVM;
  

• if (xen_hvm_domain() && xen_initial_domain())

• usage = LOCAL;
  

• if (xen_pv_domain() && !xen_start_info->store_evtchn)

• usage = LOCAL;
  

• if (xen_pv_domain() && xen_start_info->store_evtchn)

• xenstored_ready = 1;
  

• switch (usage) {

• case LOCAL:
  err = xenstored_local_init();
  if (err)
  	goto out_error;
  

• }
  

• xen_store_interface = mfn_to_virt(xen_store_mfn);
  

• 	xen_store_interface = mfn_to_virt(xen_store_mfn);
  

• 	break;
  

• case PV:
  

• 	xen_store_evtchn = xen_start_info->store_evtchn;
  

• 	xen_store_mfn = xen_start_info->store_mfn;
  

• 	xen_store_interface = mfn_to_virt(xen_store_mfn);
  

• 	break;
  

• case HVM:
  

• 	err = hvm_get_parameter(HVM_PARAM_STORE_EVTCHN, &v);
  

• 	if (err)
  

• 		goto out_error;
  

• 	xen_store_evtchn = (int)v;
  

• 	err = hvm_get_parameter(HVM_PARAM_STORE_PFN, &v);
  

• 	if (err)
  

• 		goto out_error;
  

• 	xen_store_mfn = (unsigned long)v;
  

• 	xen_store_interface =
  

• 		ioremap(xen_store_mfn << PAGE_SHIFT, PAGE_SIZE);
  

• 	break;
  

• default:
  

• 	pr_warn("Xenstore state unknown\n");
  

• 	break;
  

  }

/* Initialize the interface to xenstore. */ diff --git a/drivers/xen/xenbus/xenbus_xs.c b/drivers/xen/xenbus/xenbus_xs.c index bce15cf..131dec0 100644 --- a/drivers/xen/xenbus/xenbus_xs.c +++ b/drivers/xen/xenbus/xenbus_xs.c @@ -44,6 +44,7 @@ #include <linux/rwsem.h> #include <linux/module.h> #include <linux/mutex.h> +#include <asm/xen/hypervisor.h> #include <xen/xenbus.h> #include <xen/xen.h> #include "xenbus_comms.h" @@ -622,7 +623,7 @@ static void xs_reset_watches(void) { int err, supported = 0;

• if (!xen_hvm_domain())

• if (!xen_hvm_domain() || xen_initial_domain()) return;

err = xenbus_scanf(XBT_NIL, "control", -- 1.7.2.5

On Mon, 17 Sep 2012, Konrad Rzeszutek Wilk wrote:

On Fri, Sep 14, 2012 at 12:13:12PM +0100, Stefano Stabellini wrote:

...

bind_evtchn_to_irqhandler can legitimately return 0 (irq 0): it is not an error.

If Linux is running as an HVM domain and is running as Dom0, use xenstored_local_init to initialize the xenstore page and event channel.

Changes in v4:

• do not xs_reset_watches on dom0.

Changes in v2:

• refactor xenbus_init.

Signed-off-by: Stefano Stabellini stefano.stabellini@eu.citrix.com

Acked-by: Konrad Rzeszutek Wilk konrad.wilk@oracle.com

If you would like I can also carry this in my tree.

OK, let's do that. I'll rebase again on your tree with this patch.

...

drivers/xen/xenbus/xenbus_comms.c | 2 +- drivers/xen/xenbus/xenbus_probe.c | 62 +++++++++++++++++++++++++----------- drivers/xen/xenbus/xenbus_xs.c | 3 +- 3 files changed, 46 insertions(+), 21 deletions(-)

diff --git a/drivers/xen/xenbus/xenbus_comms.c b/drivers/xen/xenbus/xenbus_comms.c index 52fe7ad..c5aa55c 100644 --- a/drivers/xen/xenbus/xenbus_comms.c +++ b/drivers/xen/xenbus/xenbus_comms.c @@ -224,7 +224,7 @@ int xb_init_comms(void) int err; err = bind_evtchn_to_irqhandler(xen_store_evtchn, wake_waiting, 0, "xenbus", &xb_waitq);

• if (err <= 0) {
  

• if (err < 0) {
  printk(KERN_ERR "XENBUS request irq failed %i\n", err);
  return err;
  

  }

diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c index b793723..a67ccc0 100644 --- a/drivers/xen/xenbus/xenbus_probe.c +++ b/drivers/xen/xenbus/xenbus_probe.c @@ -719,37 +719,61 @@ static int __init xenstored_local_init(void) return err; } +enum xenstore_init {

• UNKNOWN,
• PV,
• HVM,
• LOCAL,

+}; static int __init xenbus_init(void) { int err = 0;

• enum xenstore_init usage = UNKNOWN;
• uint64_t v = 0;

if (!xen_domain()) return -ENODEV; xenbus_ring_ops_init();

• if (xen_hvm_domain()) {

• uint64_t v = 0;
  

• err = hvm_get_parameter(HVM_PARAM_STORE_EVTCHN, &v);
  

• if (err)
  

• 	goto out_error;
  

• xen_store_evtchn = (int)v;
  

• err = hvm_get_parameter(HVM_PARAM_STORE_PFN, &v);
  

• if (err)
  

• 	goto out_error;
  

• xen_store_mfn = (unsigned long)v;
  

• xen_store_interface = ioremap(xen_store_mfn << PAGE_SHIFT, PAGE_SIZE);
  

• } else {

• xen_store_evtchn = xen_start_info->store_evtchn;
  

• xen_store_mfn = xen_start_info->store_mfn;
  

• if (xen_store_evtchn)
  

• 	xenstored_ready = 1;
  

• else {
  

• if (xen_pv_domain())

• usage = PV;
  

• if (xen_hvm_domain())

• usage = HVM;
  

• if (xen_hvm_domain() && xen_initial_domain())

• usage = LOCAL;
  

• if (xen_pv_domain() && !xen_start_info->store_evtchn)

• usage = LOCAL;
  

• if (xen_pv_domain() && xen_start_info->store_evtchn)

• xenstored_ready = 1;
  

• switch (usage) {

• case LOCAL:
  err = xenstored_local_init();
  if (err)
  	goto out_error;
  

• }
  

• xen_store_interface = mfn_to_virt(xen_store_mfn);
  

• 	xen_store_interface = mfn_to_virt(xen_store_mfn);
  

• 	break;
  

• case PV:
  

• 	xen_store_evtchn = xen_start_info->store_evtchn;
  

• 	xen_store_mfn = xen_start_info->store_mfn;
  

• 	xen_store_interface = mfn_to_virt(xen_store_mfn);
  

• 	break;
  

• case HVM:
  

• 	err = hvm_get_parameter(HVM_PARAM_STORE_EVTCHN, &v);
  

• 	if (err)
  

• 		goto out_error;
  

• 	xen_store_evtchn = (int)v;
  

• 	err = hvm_get_parameter(HVM_PARAM_STORE_PFN, &v);
  

• 	if (err)
  

• 		goto out_error;
  

• 	xen_store_mfn = (unsigned long)v;
  

• 	xen_store_interface =
  

• 		ioremap(xen_store_mfn << PAGE_SHIFT, PAGE_SIZE);
  

• 	break;
  

• default:
  

• 	pr_warn("Xenstore state unknown\n");
  

• 	break;
  

  }

/* Initialize the interface to xenstore. */ diff --git a/drivers/xen/xenbus/xenbus_xs.c b/drivers/xen/xenbus/xenbus_xs.c index bce15cf..131dec0 100644 --- a/drivers/xen/xenbus/xenbus_xs.c +++ b/drivers/xen/xenbus/xenbus_xs.c @@ -44,6 +44,7 @@ #include <linux/rwsem.h> #include <linux/module.h> #include <linux/mutex.h> +#include <asm/xen/hypervisor.h> #include <xen/xenbus.h> #include <xen/xen.h> #include "xenbus_comms.h" @@ -622,7 +623,7 @@ static void xs_reset_watches(void) { int err, supported = 0;

• if (!xen_hvm_domain())

• if (!xen_hvm_domain() || xen_initial_domain()) return;

err = xenbus_scanf(XBT_NIL, "control", -- 1.7.2.5

Hello.

On 09/14/2012 03:13 PM, Stefano Stabellini wrote:

Changes in v2:

• mark Xen guest support on ARM as EXPERIMENTAL.

Signed-off-by: Stefano Stabellini stefano.stabellini@eu.citrix.com Acked-by: Konrad Rzeszutek Wilk konrad.wilk@oracle.com

---

arch/arm/Kconfig | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 2f88d8d..e92518d 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -1897,6 +1897,16 @@ config DEPRECATED_PARAM_STRUCT This was deprecated in 2001 and announced to live on for 5 years. Some old boot loaders still use this way. +config XEN_DOM0

• def_bool y

+config XEN

• bool "Xen guest support on ARM (EXPERIMENTAL)"
• depends on EXPERIMENTAL && ARM && OF
• select XEN_DOM0

What's the point of selecting it if it's always "y"?

WBR, Sergei

Hello.

On 17-09-2012 14:57, Stefano Stabellini wrote:

...

...

Changes in v2:

...

...

• mark Xen guest support on ARM as EXPERIMENTAL.

...

...

Signed-off-by: Stefano Stabellini stefano.stabellini@eu.citrix.com Acked-by: Konrad Rzeszutek Wilk konrad.wilk@oracle.com

---

arch/arm/Kconfig | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-)

...

...

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 2f88d8d..e92518d 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -1897,6 +1897,16 @@ config DEPRECATED_PARAM_STRUCT This was deprecated in 2001 and announced to live on for 5 years. Some old boot loaders still use this way.

+config XEN_DOM0

• def_bool y

+config XEN

• bool "Xen guest support on ARM (EXPERIMENTAL)"
• depends on EXPERIMENTAL && ARM && OF
• select XEN_DOM0

...

What's the point of selecting it if it's always "y"?

That's because on X86 is not always "y": there are things under drivers/xen that compile on both platforms and depend on XEN_DOM0.

But we're not on x86. On ARM this select is pointless.

WBR, Sergei

On Mon, 17 Sep 2012, Konrad Rzeszutek Wilk wrote:

On Mon, Sep 17, 2012 at 04:07:37PM +0400, Sergei Shtylyov wrote:

...

Hello.

On 17-09-2012 14:57, Stefano Stabellini wrote:

...

...

...

Changes in v2:

...

...

...

• mark Xen guest support on ARM as EXPERIMENTAL.

...

...

...

Signed-off-by: Stefano Stabellini stefano.stabellini@eu.citrix.com Acked-by: Konrad Rzeszutek Wilk konrad.wilk@oracle.com

---

arch/arm/Kconfig | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-)

...

...

...

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 2f88d8d..e92518d 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -1897,6 +1897,16 @@ config DEPRECATED_PARAM_STRUCT This was deprecated in 2001 and announced to live on for 5 years. Some old boot loaders still use this way.

+config XEN_DOM0

• def_bool y

+config XEN

• bool "Xen guest support on ARM (EXPERIMENTAL)"
• depends on EXPERIMENTAL && ARM && OF

I think the CONFIG_EXPERIMENTAL is going away. Or it has already gone away?

I would like to keep it for Linux v3.6, because the Xen ABI offered by Xen 4.2 hasn't been declared stable yet.

On Mon, 17 Sep 2012, Stefano Stabellini wrote:

On Mon, 17 Sep 2012, Sergei Shtylyov wrote:

...

Hello.

On 17-09-2012 14:57, Stefano Stabellini wrote:

...

...

...

Changes in v2:

...

...

...

• mark Xen guest support on ARM as EXPERIMENTAL.

...

...

...

Signed-off-by: Stefano Stabellini stefano.stabellini@eu.citrix.com Acked-by: Konrad Rzeszutek Wilk konrad.wilk@oracle.com

---

arch/arm/Kconfig | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-)

...

...

...

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 2f88d8d..e92518d 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -1897,6 +1897,16 @@ config DEPRECATED_PARAM_STRUCT This was deprecated in 2001 and announced to live on for 5 years. Some old boot loaders still use this way.

+config XEN_DOM0

• def_bool y

+config XEN

• bool "Xen guest support on ARM (EXPERIMENTAL)"
• depends on EXPERIMENTAL && ARM && OF
• select XEN_DOM0

...

...

What's the point of selecting it if it's always "y"?

...

That's because on X86 is not always "y": there are things under drivers/xen that compile on both platforms and depend on XEN_DOM0.

But we're not on x86. On ARM this select is pointless.

We need some common code (under drivers/xen) that compiles only if XEN_DOM0 is selected, so it is not pointless after all.

XEN_DOM0 is not the only symbol that is conditionally compiled on one architectuire and always "y" on another...

Wait a sec, I have just realized that written this way XEN_DOM0 is always "y", even if XEN is not! The right way of doing this is:

config XEN_DOM0 def_bool y depends on XEN

config XEN bool "Xen guest support on ARM (EXPERIMENTAL)" depends on EXPERIMENTAL && ARM && OF help Say Y if you want to run Linux in a Virtual Machine on Xen on ARM.

I am not sure if this is what you meant, but thanks for making me realize this mistake anyway! :)

On Fri, 14 Sep 2012, Konrad Rzeszutek Wilk wrote:

On Fri, Sep 14, 2012 at 12:13:16PM +0100, Stefano Stabellini wrote:

...

Initialize the grant table mapping at the address specified at index 0 in the DT under the /xen node. After the grant table is initialized, call xenbus_probe (if not dom0).

So we don't really care about the grant's size then? The DT xen.txt talks about it..

I am assuming that the size of the memory region specified in the device tree is sufficiently large to map the entire grant table, given that both the device tree hypervisor entry and the grant table size comes from Xen.

The grant table size is currently queried to Xen directly via an hypercall (GNTTABOP_query_size). Basically the size in the device tree is redundant information.

...

Changes in v2:

• introduce GRANT_TABLE_PHYSADDR;
• remove unneeded initialization of boot_max_nr_grant_frames.

Signed-off-by: Stefano Stabellini stefano.stabellini@eu.citrix.com

arch/arm/xen/enlighten.c | 14 ++++++++++++++ 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c index c2a47a7..036a4d8 100644 --- a/arch/arm/xen/enlighten.c +++ b/arch/arm/xen/enlighten.c @@ -1,8 +1,12 @@ #include <xen/xen.h> +#include <xen/grant_table.h> +#include <xen/hvm.h> #include <xen/interface/xen.h> #include <xen/interface/memory.h> +#include <xen/interface/hvm/params.h> #include <xen/features.h> #include <xen/platform_pci.h> +#include <xen/xenbus.h> #include <asm/xen/hypervisor.h> #include <asm/xen/hypercall.h> #include <linux/module.h> @@ -42,6 +46,7 @@ EXPORT_SYMBOL_GPL(xen_remap_domain_mfn_range);

• see Documentation/devicetree/bindings/arm/xen.txt for the
• documentation of the Xen Device Tree format.

*/ +#define GRANT_TABLE_PHYSADDR 0 static int __init xen_guest_init(void) { struct xen_add_to_physmap xatp; @@ -51,6 +56,7 @@ static int __init xen_guest_init(void) const char *s = NULL; const char *version = NULL; const char *xen_prefix = "xen,xen-";

• struct resource res;

node = of_find_compatible_node(NULL, NULL, "xen,xen"); if (!node) { @@ -65,6 +71,9 @@ static int __init xen_guest_init(void) pr_debug("Xen version not found\n"); return 0; }

• if (of_address_to_resource(node, GRANT_TABLE_PHYSADDR, &res))

• return 0;
  

• xen_hvm_resume_frames = res.start >> PAGE_SHIFT; xen_domain_type = XEN_HVM_DOMAIN;

xen_setup_features(); @@ -98,6 +107,11 @@ static int __init xen_guest_init(void) * is required to use VCPUOP_register_vcpu_info to place vcpu info * for secondary CPUs as they are brought up. */ per_cpu(xen_vcpu, 0) = &HYPERVISOR_shared_info->vcpu_info[0];

• gnttab_init();
• if (!xen_initial_domain())

• xenbus_probe(NULL);
  

• return 0;

} core_initcall(xen_guest_init); -- 1.7.2.5

On Fri, 14 Sep 2012, Marc Zyngier wrote:

On 14/09/12 12:13, Stefano Stabellini wrote:

...

Reset the IRQ_NOAUTOEN and IRQ_NOREQUEST flags that are enabled by default on ARM. If IRQ_NOAUTOEN is set, __setup_irq doesn't call irq_startup, that is responsible for calling irq_unmask at startup time. As a result event channels remain masked.

Signed-off-by: Stefano Stabellini stefano.stabellini@eu.citrix.com Acked-by: Konrad Rzeszutek Wilk konrad.wilk@oracle.com

---

drivers/xen/events.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/xen/events.c b/drivers/xen/events.c index 5ecb596..8ffb7b7 100644 --- a/drivers/xen/events.c +++ b/drivers/xen/events.c @@ -836,6 +836,7 @@ int bind_evtchn_to_irq(unsigned int evtchn) struct irq_info *info = info_for_irq(irq); WARN_ON(info == NULL || info->type != IRQT_EVTCHN); }

• irq_clear_status_flags(irq, IRQ_NOREQUEST|IRQ_NOAUTOEN);

This one just sent a shiver down my spine. Are you doing this for a PPI?

Not really: even though there is just one source of event notifications (that is a PPI), we have many event channels. When a domain receives a notification (via the PPI), it checks on a bitmask to which event channel it corresponds. From the Linux point of view every event channel is a Linux irq belonging to the xen_dynamic_chip (see drivers/xen/events.c:xen_dynamic_chip).

So here I am not doing this for the one PPI, but I am doing this for every Linux irq (of chip xen_dynamic_chip) that represents an event channel.

On Fri, 14 Sep 2012, Marc Zyngier wrote:

On 14/09/12 15:13, Stefano Stabellini wrote:

...

On Fri, 14 Sep 2012, Marc Zyngier wrote:

...

On 14/09/12 12:13, Stefano Stabellini wrote:

...

Reset the IRQ_NOAUTOEN and IRQ_NOREQUEST flags that are enabled by default on ARM. If IRQ_NOAUTOEN is set, __setup_irq doesn't call irq_startup, that is responsible for calling irq_unmask at startup time. As a result event channels remain masked.

Signed-off-by: Stefano Stabellini stefano.stabellini@eu.citrix.com Acked-by: Konrad Rzeszutek Wilk konrad.wilk@oracle.com

---

drivers/xen/events.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/xen/events.c b/drivers/xen/events.c index 5ecb596..8ffb7b7 100644 --- a/drivers/xen/events.c +++ b/drivers/xen/events.c @@ -836,6 +836,7 @@ int bind_evtchn_to_irq(unsigned int evtchn) struct irq_info *info = info_for_irq(irq); WARN_ON(info == NULL || info->type != IRQT_EVTCHN); }

• irq_clear_status_flags(irq, IRQ_NOREQUEST|IRQ_NOAUTOEN);

This one just sent a shiver down my spine. Are you doing this for a PPI?

Not really: even though there is just one source of event notifications (that is a PPI), we have many event channels. When a domain receives a notification (via the PPI), it checks on a bitmask to which event channel it corresponds. From the Linux point of view every event channel is a Linux irq belonging to the xen_dynamic_chip (see drivers/xen/events.c:xen_dynamic_chip).

So here I am not doing this for the one PPI, but I am doing this for every Linux irq (of chip xen_dynamic_chip) that represents an event channel.

So this is some sort of secondary interrupt controller, cascaded into your GIC emulation,

I guess it could be seen as a secondary interrupt controller

and this patch only affects the xen_dynamic_chip?

Yep

On Fri, 2012-09-14 at 12:13 +0100, Stefano Stabellini wrote:

Signed-off-by: Stefano Stabellini stefano.stabellini@eu.citrix.com CC: Russell King linux@arm.linux.org.uk CC: Pawel Moll pawel.moll@arm.com CC: Marc Zyngier marc.zyngier@arm.com

---

arch/arm/mach-vexpress/v2m.c | 11 ++++++----- 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-vexpress/v2m.c b/arch/arm/mach-vexpress/v2m.c index 37608f2..4e567f7 100644 --- a/arch/arm/mach-vexpress/v2m.c +++ b/arch/arm/mach-vexpress/v2m.c @@ -621,16 +621,17 @@ static void __init v2m_dt_timer_init(void) v2m_clk_init();

• err = of_property_read_string(of_aliases, "arm,v2m_timer", &path);
• if (WARN_ON(err))

• return;
  

• node = of_find_node_by_path(path);
• v2m_sp804_init(of_iomap(node, 0), irq_of_parse_and_map(node, 0)); if (arch_timer_of_register() != 0) twd_local_timer_of_register();

if (arch_timer_sched_clock_init() != 0) versatile_sched_clock_init(v2m_sysreg_base + V2M_SYS_24MHZ, 24000000);

• err = of_property_read_string(of_aliases, "arm,v2m_timer", &path);
• if (WARN_ON(err))

• return;
  

• node = of_find_node_by_path(path);
• v2m_sp804_init(of_iomap(node, 0), irq_of_parse_and_map(node, 0));

} static struct sys_timer v2m_dt_timer = {

Fair point. The alias is going to disappear anyway (I'm working on a VE platform rework right now), but in case I won't get it on time for 3.7, I'll make sure this one is merged instead.

Cheers!

Paweł

On Fri, 2012-09-14 at 13:48 +0100, Stefano Stabellini wrote:

On Fri, 14 Sep 2012, Pawel Moll wrote:

...

On Fri, 2012-09-14 at 12:13 +0100, Stefano Stabellini wrote:

...

Signed-off-by: Stefano Stabellini stefano.stabellini@eu.citrix.com CC: Russell King linux@arm.linux.org.uk CC: Pawel Moll pawel.moll@arm.com CC: Marc Zyngier marc.zyngier@arm.com

---

arch/arm/mach-vexpress/v2m.c | 11 ++++++----- 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-vexpress/v2m.c b/arch/arm/mach-vexpress/v2m.c index 37608f2..4e567f7 100644 --- a/arch/arm/mach-vexpress/v2m.c +++ b/arch/arm/mach-vexpress/v2m.c @@ -621,16 +621,17 @@ static void __init v2m_dt_timer_init(void) v2m_clk_init();

• err = of_property_read_string(of_aliases, "arm,v2m_timer", &path);
• if (WARN_ON(err))

• return;
  

• node = of_find_node_by_path(path);
• v2m_sp804_init(of_iomap(node, 0), irq_of_parse_and_map(node, 0)); if (arch_timer_of_register() != 0) twd_local_timer_of_register();

if (arch_timer_sched_clock_init() != 0) versatile_sched_clock_init(v2m_sysreg_base + V2M_SYS_24MHZ, 24000000);

• err = of_property_read_string(of_aliases, "arm,v2m_timer", &path);
• if (WARN_ON(err))

• return;
  

• node = of_find_node_by_path(path);
• v2m_sp804_init(of_iomap(node, 0), irq_of_parse_and_map(node, 0));

} static struct sys_timer v2m_dt_timer = {

Fair point. The alias is going to disappear anyway (I'm working on a VE platform rework right now), but in case I won't get it on time for 3.7, I'll make sure this one is merged instead.

Great, thanks! Should I leave this patch out of the Xen on ARM series for 3.7 then?

Em, I think so - it would conflict with my changes if they go in. Either way you'll have the problem sorted in 3.7.

Paweł

On 07/01/13 17:21, Stefano Stabellini wrote:

On Fri, 14 Sep 2012, Stefano Stabellini wrote:

...

On Fri, 14 Sep 2012, Pawel Moll wrote:

...

On Fri, 2012-09-14 at 12:13 +0100, Stefano Stabellini wrote:

...

Signed-off-by: Stefano Stabellini stefano.stabellini@eu.citrix.com CC: Russell King linux@arm.linux.org.uk CC: Pawel Moll pawel.moll@arm.com CC: Marc Zyngier marc.zyngier@arm.com

---

arch/arm/mach-vexpress/v2m.c | 11 ++++++----- 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-vexpress/v2m.c b/arch/arm/mach-vexpress/v2m.c index 37608f2..4e567f7 100644 --- a/arch/arm/mach-vexpress/v2m.c +++ b/arch/arm/mach-vexpress/v2m.c @@ -621,16 +621,17 @@ static void __init v2m_dt_timer_init(void) v2m_clk_init();

• err = of_property_read_string(of_aliases, "arm,v2m_timer", &path);
• if (WARN_ON(err))

• return;
  

• node = of_find_node_by_path(path);
• v2m_sp804_init(of_iomap(node, 0), irq_of_parse_and_map(node, 0)); if (arch_timer_of_register() != 0) twd_local_timer_of_register();

if (arch_timer_sched_clock_init() != 0) versatile_sched_clock_init(v2m_sysreg_base + V2M_SYS_24MHZ, 24000000);

• err = of_property_read_string(of_aliases, "arm,v2m_timer", &path);
• if (WARN_ON(err))

• return;
  

• node = of_find_node_by_path(path);
• v2m_sp804_init(of_iomap(node, 0), irq_of_parse_and_map(node, 0));

} static struct sys_timer v2m_dt_timer = {

Fair point. The alias is going to disappear anyway (I'm working on a VE platform rework right now), but in case I won't get it on time for 3.7, I'll make sure this one is merged instead.

Great, thanks! Should I leave this patch out of the Xen on ARM series for 3.7 then?

I have just noticed that this patch hasn't been merged yet, but still we need it to boot Linux on Xen. Do you want me to rebase and resent?

I'm looking at v3.8-rc2, and the code have changed significantly (see 433683a66401adb0150792e725cc4f631c94de46).

If you don't have an "arm,sp804" device in your DT, it shouldn't get registered and carry on registering the per-cpu timers. Can you check that it fixes your issue?

Thanks,

M.

On Fri, Sep 14, 2012 at 12:13:25PM +0100, Stefano Stabellini wrote:

Update struct xen_add_to_physmap to be in sync with Xen's version of the structure. The size field was introduced by:

changeset: 24164:707d27fe03e7 user: Jean Guyader jean.guyader@eu.citrix.com date: Fri Nov 18 13:42:08 2011 +0000 summary: mm: New XENMEM space, XENMAPSPACE_gmfn_range

According to the comment:

"This new field .size is located in the 16 bits padding between .domid and .space in struct xen_add_to_physmap to stay compatible with older versions."

Note: this patch should be already in Konrad's tree, it is here just for convenience.

Changes in v2:

• remove erroneous comment in the commit message.

Signed-off-by: Stefano Stabellini stefano.stabellini@eu.citrix.com

Acked-by: Konrad Rzeszutek Wilk konrad.wilk@oracle.com

.. and I already have this queued for 3.7.

---

include/xen/interface/memory.h | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/include/xen/interface/memory.h b/include/xen/interface/memory.h index b5c3098..b66d04c 100644 --- a/include/xen/interface/memory.h +++ b/include/xen/interface/memory.h @@ -163,6 +163,9 @@ struct xen_add_to_physmap { /* Which domain to change the mapping for. */ domid_t domid;

• /* Number of pages to go through for gmfn_range */
• uint16_t size;
• /* Source mapping space. */

#define XENMAPSPACE_shared_info 0 /* shared info page */

#define XENMAPSPACE_grant_table 1 /* grant table page */

1.7.2.5

On Fri, Sep 14, 2012 at 12:13:26PM +0100, Stefano Stabellini wrote:

Signed-off-by: Stefano Stabellini stefano.stabellini@eu.citrix.com CC: Arnd Bergmann arnd@arndb.de CC: Konrad Rzeszutek Wilk konrad.wilk@oracle.com

Acked-by: Konrad Rzeszutek Wilk konrad.wilk@oracle.com

---

MAINTAINERS | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS index fdc0119..3d38291 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7752,6 +7752,13 @@ F: drivers/xen/ F: arch/x86/include/asm/xen/ F: include/xen/ +XEN HYPERVISOR ARM +M: Stefano Stabellini stefano.stabellini@eu.citrix.com +L: xen-devel@lists.xensource.com (moderated for non-subscribers) +S: Supported +F: arch/arm/xen/ +F: arch/arm/include/asm/xen/

XEN NETWORK BACKEND DRIVER M: Ian Campbell ian.campbell@citrix.com L: xen-devel@lists.xensource.com (moderated for non-subscribers) -- 1.7.2.5