Correct DTB compatibility for xen /hypervisor node?

Correct DTB compatibility for xen /hypervisor node?

[Ian Campbell]

Looks like we have both 4.2 and 4.3 in use. Which should it be?

$ rgrep xen-4 arch/arm Documentation/
arch/arm/boot/dts/xenvm-4.2.dts:		compatible = "xen,xen-4.2", "xen,xen";
Documentation/devicetree/bindings/arm/xen.txt:	compatible = "xen,xen-4.3", "xen,xen";

FWIW I've always used 4.2 in my own DTB files.

Strictly speaking the API is the Xen 3.x API, but that doesn't seem
terribly relevant.

I suppose arch_get_xen_caps ought to return something consistent?

Ian.

Re: Correct DTB compatibility for xen /hypervisor node?

[Ian Campbell]

On Mon, 2013-01-28 at 16:50 +0000, Ian Campbell wrote:
> Looks like we have both 4.2 and 4.3 in use. Which should it be?
> 
> $ rgrep xen-4 arch/arm Documentation/
> arch/arm/boot/dts/xenvm-4.2.dts:		compatible = "xen,xen-4.2", "xen,xen";
> Documentation/devicetree/bindings/arm/xen.txt:	compatible = "xen,xen-4.3", "xen,xen";
> 
> FWIW I've always used 4.2 in my own DTB files.
> 
> Strictly speaking the API is the Xen 3.x API, but that doesn't seem
> terribly relevant.
> 
> I suppose arch_get_xen_caps ought to return something consistent?

On a related note should /hypervisor contain #interrupt-cells = <3> ?
We use "interrupts = <1 15 0xf08>" so it is somewhat implicit, butsince
the parent doesn't have anything explicit this it will default to
$somethingelse (0)? 

Should Documentation/devicetree/bindings/arm/xen.txt also reference
gic.txt for the meaning of the fields, or are we free to define our own
meaning?

We need to get a canonical copy of this binding doc into the hypervisor
tree too.

Ian.

Re: Correct DTB compatibility for xen /hypervisor node?

[Stefano Stabellini]

On Wed, 30 Jan 2013, Ian Campbell wrote:
> On Mon, 2013-01-28 at 16:50 +0000, Ian Campbell wrote:
> > Looks like we have both 4.2 and 4.3 in use. Which should it be?
> > 
> > $ rgrep xen-4 arch/arm Documentation/
> > arch/arm/boot/dts/xenvm-4.2.dts:		compatible = "xen,xen-4.2", "xen,xen";
> > Documentation/devicetree/bindings/arm/xen.txt:	compatible = "xen,xen-4.3", "xen,xen";
> > 
> > FWIW I've always used 4.2 in my own DTB files.
> > 
> > Strictly speaking the API is the Xen 3.x API, but that doesn't seem
> > terribly relevant.
> > 
> > I suppose arch_get_xen_caps ought to return something consistent?
> 
> On a related note should /hypervisor contain #interrupt-cells = <3> ?
> We use "interrupts = <1 15 0xf08>" so it is somewhat implicit, butsince
> the parent doesn't have anything explicit this it will default to
> $somethingelse (0)? 

I think that it is implicit that the interrupts attributes follow the
rules specified by the interrupt controller node.


> Should Documentation/devicetree/bindings/arm/xen.txt also reference
> gic.txt for the meaning of the fields, or are we free to define our own
> meaning?

No, we are not. Referencing gic.txt is a good idea.


> We need to get a canonical copy of this binding doc into the hypervisor
> tree too.

Right.

Re: Correct DTB compatibility for xen /hypervisor node?

[Stefano Stabellini]

On Mon, 28 Jan 2013, Ian Campbell wrote:
> Looks like we have both 4.2 and 4.3 in use. Which should it be?
> 
> $ rgrep xen-4 arch/arm Documentation/
> arch/arm/boot/dts/xenvm-4.2.dts:		compatible = "xen,xen-4.2", "xen,xen";
> Documentation/devicetree/bindings/arm/xen.txt:	compatible = "xen,xen-4.3", "xen,xen";
> 
> FWIW I've always used 4.2 in my own DTB files.
> 
> Strictly speaking the API is the Xen 3.x API, but that doesn't seem
> terribly relevant.

I have always used 4.2 too.
Even though the one in xen.txt is just an example it might be worth
updating with xen-4.2 to avoid confusions.

> I suppose arch_get_xen_caps ought to return something consistent?

Right.

>From my point of view, it would be useful to return a string
that expresses the current hypervisor version (xen-4.3 from the Xen 4.3
release, xen-4.4 from the Xen 4.4 release and so on) rather than xen-3.0.
I would do the same in the compatibility node of the DTS.

Re: Correct DTB compatibility for xen /hypervisor node?

[Ian Campbell]

On Wed, 2013-01-30 at 16:42 +0000, Stefano Stabellini wrote:
> On Wed, 30 Jan 2013, Ian Campbell wrote:
> > On Mon, 2013-01-28 at 16:50 +0000, Ian Campbell wrote:
> > > Looks like we have both 4.2 and 4.3 in use. Which should it be?
> > > 
> > > $ rgrep xen-4 arch/arm Documentation/
> > > arch/arm/boot/dts/xenvm-4.2.dts:		compatible = "xen,xen-4.2", "xen,xen";
> > > Documentation/devicetree/bindings/arm/xen.txt:	compatible = "xen,xen-4.3", "xen,xen";
> > > 
> > > FWIW I've always used 4.2 in my own DTB files.
> > > 
> > > Strictly speaking the API is the Xen 3.x API, but that doesn't seem
> > > terribly relevant.
> > > 
> > > I suppose arch_get_xen_caps ought to return something consistent?
> > 
> > On a related note should /hypervisor contain #interrupt-cells = <3> ?
> > We use "interrupts = <1 15 0xf08>" so it is somewhat implicit, butsince
> > the parent doesn't have anything explicit this it will default to
> > $somethingelse (0)? 
> 
> I think that it is implicit that the interrupts attributes follow the
> rules specified by the interrupt controller node.

OK.

> > Should Documentation/devicetree/bindings/arm/xen.txt also reference
> > gic.txt for the meaning of the fields, or are we free to define our own
> > meaning?
> 
> No, we are not. Referencing gic.txt is a good idea.

Yes, especially since we assume that interrupt-controller == gic.

> > We need to get a canonical copy of this binding doc into the hypervisor
> > tree too.
> 
> Right.

Are you going to send a patch to both Linux and Xen?

Ian.

Re: Correct DTB compatibility for xen /hypervisor node?

[Ian Campbell]

On Wed, 2013-01-30 at 16:49 +0000, Stefano Stabellini wrote:
> On Mon, 28 Jan 2013, Ian Campbell wrote:
> > Looks like we have both 4.2 and 4.3 in use. Which should it be?
> > 
> > $ rgrep xen-4 arch/arm Documentation/
> > arch/arm/boot/dts/xenvm-4.2.dts:		compatible = "xen,xen-4.2", "xen,xen";
> > Documentation/devicetree/bindings/arm/xen.txt:	compatible = "xen,xen-4.3", "xen,xen";
> > 
> > FWIW I've always used 4.2 in my own DTB files.
> > 
> > Strictly speaking the API is the Xen 3.x API, but that doesn't seem
> > terribly relevant.
> 
> I have always used 4.2 too.
> Even though the one in xen.txt is just an example it might be worth
> updating with xen-4.2 to avoid confusions.

Or maybe we should update to 4.3 everywhere.

> > I suppose arch_get_xen_caps ought to return something consistent?
> 
> Right.
> 
> From my point of view, it would be useful to return a string
> that expresses the current hypervisor version (xen-4.3 from the Xen 4.3
> release, xen-4.4 from the Xen 4.4 release and so on) rather than xen-3.0.
> I would do the same in the compatibility node of the DTS.

Remember that the capability string returned by the hypervisor is not
exposing the version of the hypervisor -- it is exposing the version of
the Hypervisor *ABI*, which is a subtly different thing.

I think we should follow the hypervisor version until we freeze the ABI,
at which point we hardcode whichever version we froze at?

Ian.

Re: Correct DTB compatibility for xen /hypervisor node?

[Stefano Stabellini]

On Thu, 31 Jan 2013, Ian Campbell wrote:
> On Wed, 2013-01-30 at 16:49 +0000, Stefano Stabellini wrote:
> > On Mon, 28 Jan 2013, Ian Campbell wrote:
> > > Looks like we have both 4.2 and 4.3 in use. Which should it be?
> > > 
> > > $ rgrep xen-4 arch/arm Documentation/
> > > arch/arm/boot/dts/xenvm-4.2.dts:		compatible = "xen,xen-4.2", "xen,xen";
> > > Documentation/devicetree/bindings/arm/xen.txt:	compatible = "xen,xen-4.3", "xen,xen";
> > > 
> > > FWIW I've always used 4.2 in my own DTB files.
> > > 
> > > Strictly speaking the API is the Xen 3.x API, but that doesn't seem
> > > terribly relevant.
> > 
> > I have always used 4.2 too.
> > Even though the one in xen.txt is just an example it might be worth
> > updating with xen-4.2 to avoid confusions.
> 
> Or maybe we should update to 4.3 everywhere.
> 
> > > I suppose arch_get_xen_caps ought to return something consistent?
> > 
> > Right.
> > 
> > From my point of view, it would be useful to return a string
> > that expresses the current hypervisor version (xen-4.3 from the Xen 4.3
> > release, xen-4.4 from the Xen 4.4 release and so on) rather than xen-3.0.
> > I would do the same in the compatibility node of the DTS.
> 
> Remember that the capability string returned by the hypervisor is not
> exposing the version of the hypervisor -- it is exposing the version of
> the Hypervisor *ABI*, which is a subtly different thing.
> 
> I think we should follow the hypervisor version until we freeze the ABI,
> at which point we hardcode whichever version we froze at?

That's reasonable, but we shouldn't be afraid of bumping that number.

Re: Correct DTB compatibility for xen /hypervisor node?

[Ian Campbell]

On Thu, 2013-01-31 at 11:53 +0000, Stefano Stabellini wrote:
> On Thu, 31 Jan 2013, Ian Campbell wrote:
> > On Wed, 2013-01-30 at 16:49 +0000, Stefano Stabellini wrote:
> > > On Mon, 28 Jan 2013, Ian Campbell wrote:
> > > > Looks like we have both 4.2 and 4.3 in use. Which should it be?
> > > > 
> > > > $ rgrep xen-4 arch/arm Documentation/
> > > > arch/arm/boot/dts/xenvm-4.2.dts:		compatible = "xen,xen-4.2", "xen,xen";
> > > > Documentation/devicetree/bindings/arm/xen.txt:	compatible = "xen,xen-4.3", "xen,xen";
> > > > 
> > > > FWIW I've always used 4.2 in my own DTB files.
> > > > 
> > > > Strictly speaking the API is the Xen 3.x API, but that doesn't seem
> > > > terribly relevant.
> > > 
> > > I have always used 4.2 too.
> > > Even though the one in xen.txt is just an example it might be worth
> > > updating with xen-4.2 to avoid confusions.
> > 
> > Or maybe we should update to 4.3 everywhere.
> > 
> > > > I suppose arch_get_xen_caps ought to return something consistent?
> > > 
> > > Right.
> > > 
> > > From my point of view, it would be useful to return a string
> > > that expresses the current hypervisor version (xen-4.3 from the Xen 4.3
> > > release, xen-4.4 from the Xen 4.4 release and so on) rather than xen-3.0.
> > > I would do the same in the compatibility node of the DTS.
> > 
> > Remember that the capability string returned by the hypervisor is not
> > exposing the version of the hypervisor -- it is exposing the version of
> > the Hypervisor *ABI*, which is a subtly different thing.
> > 
> > I think we should follow the hypervisor version until we freeze the ABI,
> > at which point we hardcode whichever version we froze at?
> 
> That's reasonable, but we shouldn't be afraid of bumping that number.

Once we have frozen the ABI we should be very afraid of bumping that
number, because it would indicate that old guests might not run...

Ian.

Re: Correct DTB compatibility for xen /hypervisor node?

[Stefano Stabellini]

On Thu, 31 Jan 2013, Ian Campbell wrote:
> On Thu, 2013-01-31 at 11:53 +0000, Stefano Stabellini wrote:
> > On Thu, 31 Jan 2013, Ian Campbell wrote:
> > > On Wed, 2013-01-30 at 16:49 +0000, Stefano Stabellini wrote:
> > > > On Mon, 28 Jan 2013, Ian Campbell wrote:
> > > > > Looks like we have both 4.2 and 4.3 in use. Which should it be?
> > > > > 
> > > > > $ rgrep xen-4 arch/arm Documentation/
> > > > > arch/arm/boot/dts/xenvm-4.2.dts:		compatible = "xen,xen-4.2", "xen,xen";
> > > > > Documentation/devicetree/bindings/arm/xen.txt:	compatible = "xen,xen-4.3", "xen,xen";
> > > > > 
> > > > > FWIW I've always used 4.2 in my own DTB files.
> > > > > 
> > > > > Strictly speaking the API is the Xen 3.x API, but that doesn't seem
> > > > > terribly relevant.
> > > > 
> > > > I have always used 4.2 too.
> > > > Even though the one in xen.txt is just an example it might be worth
> > > > updating with xen-4.2 to avoid confusions.
> > > 
> > > Or maybe we should update to 4.3 everywhere.
> > > 
> > > > > I suppose arch_get_xen_caps ought to return something consistent?
> > > > 
> > > > Right.
> > > > 
> > > > From my point of view, it would be useful to return a string
> > > > that expresses the current hypervisor version (xen-4.3 from the Xen 4.3
> > > > release, xen-4.4 from the Xen 4.4 release and so on) rather than xen-3.0.
> > > > I would do the same in the compatibility node of the DTS.
> > > 
> > > Remember that the capability string returned by the hypervisor is not
> > > exposing the version of the hypervisor -- it is exposing the version of
> > > the Hypervisor *ABI*, which is a subtly different thing.
> > > 
> > > I think we should follow the hypervisor version until we freeze the ABI,
> > > at which point we hardcode whichever version we froze at?
> > 
> > That's reasonable, but we shouldn't be afraid of bumping that number.
> 
> Once we have frozen the ABI we should be very afraid of bumping that
> number, because it would indicate that old guests might not run...

OK.

In that case I suggest that we should NOT expose the hypercall ABI in
the compatible node of the DTS, but the hypervisor version.
After all, like you pointed out, the guest already has a way to get the
hypercall ABI version, it doesn't need two. It gives us more flexibility
and it is certainly going to turn out useful in the future.

Re: Correct DTB compatibility for xen /hypervisor node?

[Stefano Stabellini]

On Thu, 31 Jan 2013, Ian Campbell wrote:
> On Wed, 2013-01-30 at 16:42 +0000, Stefano Stabellini wrote:
> > On Wed, 30 Jan 2013, Ian Campbell wrote:
> > > On Mon, 2013-01-28 at 16:50 +0000, Ian Campbell wrote:
> > > > Looks like we have both 4.2 and 4.3 in use. Which should it be?
> > > > 
> > > > $ rgrep xen-4 arch/arm Documentation/
> > > > arch/arm/boot/dts/xenvm-4.2.dts:		compatible = "xen,xen-4.2", "xen,xen";
> > > > Documentation/devicetree/bindings/arm/xen.txt:	compatible = "xen,xen-4.3", "xen,xen";
> > > > 
> > > > FWIW I've always used 4.2 in my own DTB files.
> > > > 
> > > > Strictly speaking the API is the Xen 3.x API, but that doesn't seem
> > > > terribly relevant.
> > > > 
> > > > I suppose arch_get_xen_caps ought to return something consistent?
> > > 
> > > On a related note should /hypervisor contain #interrupt-cells = <3> ?
> > > We use "interrupts = <1 15 0xf08>" so it is somewhat implicit, butsince
> > > the parent doesn't have anything explicit this it will default to
> > > $somethingelse (0)? 
> > 
> > I think that it is implicit that the interrupts attributes follow the
> > rules specified by the interrupt controller node.
> 
> OK.
> 
> > > Should Documentation/devicetree/bindings/arm/xen.txt also reference
> > > gic.txt for the meaning of the fields, or are we free to define our own
> > > meaning?
> > 
> > No, we are not. Referencing gic.txt is a good idea.
> 
> Yes, especially since we assume that interrupt-controller == gic.
> 
> > > We need to get a canonical copy of this binding doc into the hypervisor
> > > tree too.
> > 
> > Right.
> 
> Are you going to send a patch to both Linux and Xen?

I'll add this to my todo list