Xen devel - Jan 2013 - Correct DTB compatibility for xen /hypervisor node?

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.

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.

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.

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.

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.

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.

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.

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.

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.

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