Re: [Xen-devel] [PATCH v5.99.1 RFC 1/4] xen/arm: Duplicate gic-v2.c file to support hip04 platform version
On Thu, 26 Feb 2015, Ian Campbell wrote: > On Wed, 2015-02-25 at 16:34 +, Stefano Stabellini wrote: > > I think we should disable the build of all drivers in Xen by default, > > except for the ARM standard compliant ones (for aarch64 the SBSA is a > > nice summary of what is considered compliant), to keep the size of the > > binary small. > > I think this last statement was based on information that the gic-v2 > driver was of the order of 70-100K in size, but I think that information > was wrong (I suspect it was the raw .o size, which includes debug info > and other extraneous bits). Here I see: > > $ du -h xen/arch/arm/gic-v2.o > 148K xen/arch/arm/gic-v2.o > $ aarch64-linux-gnu-size xen/arch/arm/gic-v2.o >text data bss dec hex filename >6619 0 9767161a3c xen/arch/arm/gic-v2.o > > IOW the actual binary size is on the order of 6K (gic-v3.o is around the > same). This is arm64, I can't be bothered to rebuild for arm32, it'll be > similar. > > Given that then I really don't think it is worth introducing a two tier > build over it. > > If we really cared about these sorts of savings we would arrange to > discard all of the unused GIC/SMMU/UART driver's .text/.data/.bss after > boot (easy enough to achieve by putting each in a dedicated segment). > > But I don't think we have enough such drivers to start worrying about > doing that just now. We have that opportunity in our back pocket if we > ever get to that point, which is good enough I think. > > > Could you please introduce a Xen build time option in > > xen/arch/arm/Rules.mk, called HAS_NON_STANDARD_DRIVERS, that by default > > is n, and gate the build of gic-hip04.c on it? > > Frediano, I see you've already done so in v6, thanks for that. Sorry to > go back on it. > > Assuming the rest of the series in v6 is OK (gets acked and whatever) > then I expect I can just skip that one patch when applying and fixup the > Makefile in the obvious way (approx s/HAS_NON.../CONFIG_ARM32/) in the > dependent patch. v6 is fine from my POV, you can add my Acked-by to all patches. I am OK with dropping HAS_NON_STANDARD_DRIVERS. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v5.99.1 RFC 1/4] xen/arm: Duplicate gic-v2.c file to support hip04 platform version
On Wed, 2015-02-25 at 16:34 +, Stefano Stabellini wrote: > I think we should disable the build of all drivers in Xen by default, > except for the ARM standard compliant ones (for aarch64 the SBSA is a > nice summary of what is considered compliant), to keep the size of the > binary small. I think this last statement was based on information that the gic-v2 driver was of the order of 70-100K in size, but I think that information was wrong (I suspect it was the raw .o size, which includes debug info and other extraneous bits). Here I see: $ du -h xen/arch/arm/gic-v2.o 148Kxen/arch/arm/gic-v2.o $ aarch64-linux-gnu-size xen/arch/arm/gic-v2.o textdata bss dec hex filename 6619 0 9767161a3c xen/arch/arm/gic-v2.o IOW the actual binary size is on the order of 6K (gic-v3.o is around the same). This is arm64, I can't be bothered to rebuild for arm32, it'll be similar. Given that then I really don't think it is worth introducing a two tier build over it. If we really cared about these sorts of savings we would arrange to discard all of the unused GIC/SMMU/UART driver's .text/.data/.bss after boot (easy enough to achieve by putting each in a dedicated segment). But I don't think we have enough such drivers to start worrying about doing that just now. We have that opportunity in our back pocket if we ever get to that point, which is good enough I think. > Could you please introduce a Xen build time option in > xen/arch/arm/Rules.mk, called HAS_NON_STANDARD_DRIVERS, that by default > is n, and gate the build of gic-hip04.c on it? Frediano, I see you've already done so in v6, thanks for that. Sorry to go back on it. Assuming the rest of the series in v6 is OK (gets acked and whatever) then I expect I can just skip that one patch when applying and fixup the Makefile in the obvious way (approx s/HAS_NON.../CONFIG_ARM32/) in the dependent patch. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v5.99.1 RFC 1/4] xen/arm: Duplicate gic-v2.c file to support hip04 platform version
On Thu, 2015-02-26 at 13:54 +, Julien Grall wrote:
> > NB: I'm only considering host level stuff here. Our virtualised hardware
> > as exposed to the guest is well defined right now and any conversation
> > about deviating from the set of hardware (e.g. providing a guest view to
> > a non-GIC compliant virtual interrupt controller) would be part of a
> > separate larger conversation about "hvm" style guests (and I'd, as you
> > might imagine, be very reluctant to code to Xen itself to support
> > non-standard vGICs in particular).
>
> That would mean on platform such as Hisilicon, Guest (including DOM0)
> won't be able to use more than 8 CPUs. But I guess this is a fair trade
> for having a GIC which differs from the spec.
Correct.
>
> > From a "what does 'standards compliant' mean" PoV we have:
> >
> > CPUs:
> >
> > Specified in the ARM ARM (v7=ARM DDI 0406, v8=ARM DDI 0487).
> >
> > Uncontroversial, I hope!
> >
> > Host interrupt controllers:
> >
> > Defined in "ARM Generic Interrupt Controller Architecture
> > Specification" (v2=ARM IHI 0048B, v3=???).
>
> AFAICT, for GICv3 there is a hardware spec available (though not
> publicly) but no developer spec.
The "Architecture Specification" is the one we want, I don't know if
that is what you meant by hardware spec, I have one although as you say
I don't think it is public yet.
>
> > Referenced from ARMv8 ARM (but not required AFAICT) but
> > nonetheless this is what we consider when we talk about the
> > "standard interrupt controller".
> >
> > Host timers:
> >
> > Defined in the ARMv8 ARM "Generic Timers" chapter.
> >
> > Defined as an extension to ARMv7 (don't have doc ref handy). For
> > our purposes such extensions are considered standardized[*].
>
> It's worth to mention that we don't support generic memory mapped timer
> for now. I don't know if we aim to support it.
I don't know either, yet. For now we don't, that's correct.
> > UARTS:
> >
> > SBSA defines some (pl011 only?), but there are lots, including
> > 8250-alike ones (ns16550 etc) which is a well established
> > standard (from x86).
> >
> > Given that UART drivers are generally small and pretty trivial I
> > think we can tolerate "non-standard" (i.e. non-SBSA, non-8250)
> > ones, so long as they are able to support our vuart interface.
> >
> > I think the non-{pl011,8250} ones should be subject to non-core
> > (i.e. community) maintenance as I mentioned previously, i.e
> > should be listed in MAINTAINERS other than under the core ARM
> > entry. If we decide to go ahead with this approach I'll ask the
> > appropriate people to update MAINTAINERS.
>
> At that time we have 3 "non-compliant" UART in Xen: exynos4210, scif and
> omap.
>
> Having maintainers per non-compliant UART would make some generic more
> complicate to upstream.
In reality by a negligible amount, I expect.
> Indeed, it would require all the ack.
I don't think that's true, an update to core which requires updates to
all drivers shouldn't be blocked by non-responsive maintainer. If they
don't respond then their driver might break.
This all works fine for much larger projects. Take Linux for example:
you don't see them getting stalled on core infrastructure updates
because the author of some niche serial driver isn't responding to his
mail. They do the sensible thing and get on with it.
> [..]
>
> > I think the above is a workable definition of what it is reasonable to
> > expect the core Xen/ARM maintainers to look after (with that particular
> > hat on) vs. what it should be expected for interested members of the
> > community to step up and maintain (where the people who happen to be
> > core Xen/ARM maintainers may occasionally chose to have such a hat too.)
>
> I got few questions about it:
> - What happen if the maintainers of a specific driver (UART/IRQ
> controller) doesn't answer?
Then their driver might break or bitrot, and eventually be removed.
> - How do we handle possible security issue related to a specific
> driver? Is it even considered as a security one?
In the same way we do today with any security issue, which is to say the
security team will deal with it, bringing in people as they feel
appropriate (and the discoverer agrees). This is no different to a bug
in any other bit of Xen who's maintainer is not on the security team.
> - As a new drivers would tight to a new set of maintainers, how do we
> decide that a new drivers is accepted in Xen?
In the normal way.
> Given the governance spec [1], we may decide to reject a maintainers for
> some reason. Does it mean the drivers is rejected too?
If someone writes a driver for a h/w component and wants to be the
maintainer then there is no normal reason to reject them IMHO.
To put it anoth
Re: [Xen-devel] [PATCH v5.99.1 RFC 1/4] xen/arm: Duplicate gic-v2.c file to support hip04 platform version
Hi Ian,
On 26/02/15 12:44, Ian Campbell wrote:
> The set of hardware which Xen needs to be able to drive (rather than
> give to a hardware domain) is:
>
> * CPUs
> * Host interrupt controllers
> * Host timers
> * A UART
> * Second-state MMUs
> * Second-stage SMMUs/IOMMU (First-stage used by domains)
> * PCI host bridges (in the near future)
>
> (did I forget any?)
I think everything is there.
> NB: I'm only considering host level stuff here. Our virtualised hardware
> as exposed to the guest is well defined right now and any conversation
> about deviating from the set of hardware (e.g. providing a guest view to
> a non-GIC compliant virtual interrupt controller) would be part of a
> separate larger conversation about "hvm" style guests (and I'd, as you
> might imagine, be very reluctant to code to Xen itself to support
> non-standard vGICs in particular).
That would mean on platform such as Hisilicon, Guest (including DOM0)
won't be able to use more than 8 CPUs. But I guess this is a fair trade
for having a GIC which differs from the spec.
> From a "what does 'standards compliant' mean" PoV we have:
>
> CPUs:
>
> Specified in the ARM ARM (v7=ARM DDI 0406, v8=ARM DDI 0487).
>
> Uncontroversial, I hope!
>
> Host interrupt controllers:
>
> Defined in "ARM Generic Interrupt Controller Architecture
> Specification" (v2=ARM IHI 0048B, v3=???).
AFAICT, for GICv3 there is a hardware spec available (though not
publicly) but no developer spec.
> Referenced from ARMv8 ARM (but not required AFAICT) but
> nonetheless this is what we consider when we talk about the
> "standard interrupt controller".
>
> Host timers:
>
> Defined in the ARMv8 ARM "Generic Timers" chapter.
>
> Defined as an extension to ARMv7 (don't have doc ref handy). For
> our purposes such extensions are considered standardized[*].
It's worth to mention that we don't support generic memory mapped timer
for now. I don't know if we aim to support it.
> UARTS:
>
> SBSA defines some (pl011 only?), but there are lots, including
> 8250-alike ones (ns16550 etc) which is a well established
> standard (from x86).
>
> Given that UART drivers are generally small and pretty trivial I
> think we can tolerate "non-standard" (i.e. non-SBSA, non-8250)
> ones, so long as they are able to support our vuart interface.
>
> I think the non-{pl011,8250} ones should be subject to non-core
> (i.e. community) maintenance as I mentioned previously, i.e
> should be listed in MAINTAINERS other than under the core ARM
> entry. If we decide to go ahead with this approach I'll ask the
> appropriate people to update MAINTAINERS.
At that time we have 3 "non-compliant" UART in Xen: exynos4210, scif and
omap.
Having maintainers per non-compliant UART would make some generic more
complicate to upstream. Indeed, it would require all the ack.
>
> Second stage MMU:
>
> Defined in the ARMv8 ARM, or the ARMv7 LPAE extensions[*, **].
>
> Second stage SMMU/IOMMU:
>
> Defined in "ARM System Memory Management Unit Architecture
> Specification" (ARM IHI 0062D?)
The D is the revision, so this would be ARM IHI 0062 for all the SMMUs.
[..]
> I think the above is a workable definition of what it is reasonable to
> expect the core Xen/ARM maintainers to look after (with that particular
> hat on) vs. what it should be expected for interested members of the
> community to step up and maintain (where the people who happen to be
> core Xen/ARM maintainers may occasionally chose to have such a hat too.)
I got few questions about it:
- What happen if the maintainers of a specific driver (UART/IRQ
controller) doesn't answer?
- How do we handle possible security issue related to a specific
driver? Is it even considered as a security one?
- As a new drivers would tight to a new set of maintainers, how do we
decide that a new drivers is accepted in Xen?
Given the governance spec [1], we may decide to reject a maintainers for
some reason. Does it mean the drivers is rejected too?
Overall, I think we should clearly define the condition of
acceptance/maintenance of a specific driver.
[..]
> [**] The LPAE extensions include/are mixed with the hyp mode page table
> format, so we pretty certainly need it.
Rigth, the ARM spec required LPAE extensions when virtualization is
supported.
Regards,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v5.99.1 RFC 1/4] xen/arm: Duplicate gic-v2.c file to support hip04 platform version
On Wed, 2015-02-25 at 16:59 +, Ian Campbell wrote:
> > I think we should disable the build of all drivers in Xen by default,
> > except for the ARM standard compliant ones (for aarch64 the SBSA is a
> > nice summary of what is considered compliant), to keep the size of the
> > binary small.
>
> I don't think the SBSA is necessarily the best reference here, since it
> only defines what is standardised within the scope of "server
> systems" (whatever you take that to mean) and there are things which do
> not fall under that umbrella.
>
> That said I'm not sure what better reference there is.
>
> Maybe even on non-server systems the set of hardware which Xen has to
> drive itself is limited to things which are covered by the SBSA in
> practice, by the nature of the fact that the majority of the wacky stuff
> is driven from hardware domains.
>
> So maybe SBSA is OK then...
Having thought about this overnight I'm now not so sure of this again, I
don't think SBSA is the definition we want.
Lets take a step back...
The set of hardware which Xen needs to be able to drive (rather than
give to a hardware domain) is:
* CPUs
* Host interrupt controllers
* Host timers
* A UART
* Second-state MMUs
* Second-stage SMMUs/IOMMU (First-stage used by domains)
* PCI host bridges (in the near future)
(did I forget any?)
NB: I'm only considering host level stuff here. Our virtualised hardware
as exposed to the guest is well defined right now and any conversation
about deviating from the set of hardware (e.g. providing a guest view to
a non-GIC compliant virtual interrupt controller) would be part of a
separate larger conversation about "hvm" style guests (and I'd, as you
might imagine, be very reluctant to code to Xen itself to support
non-standard vGICs in particular).
>From a "what does 'standards compliant' mean" PoV we have:
CPUs:
Specified in the ARM ARM (v7=ARM DDI 0406, v8=ARM DDI 0487).
Uncontroversial, I hope!
Host interrupt controllers:
Defined in "ARM Generic Interrupt Controller Architecture
Specification" (v2=ARM IHI 0048B, v3=???).
Referenced from ARMv8 ARM (but not required AFAICT) but
nonetheless this is what we consider when we talk about the
"standard interrupt controller".
Host timers:
Defined in the ARMv8 ARM "Generic Timers" chapter.
Defined as an extension to ARMv7 (don't have doc ref handy). For
our purposes such extensions are considered standardized[*].
UARTS:
SBSA defines some (pl011 only?), but there are lots, including
8250-alike ones (ns16550 etc) which is a well established
standard (from x86).
Given that UART drivers are generally small and pretty trivial I
think we can tolerate "non-standard" (i.e. non-SBSA, non-8250)
ones, so long as they are able to support our vuart interface.
I think the non-{pl011,8250} ones should be subject to non-core
(i.e. community) maintenance as I mentioned previously, i.e
should be listed in MAINTAINERS other than under the core ARM
entry. If we decide to go ahead with this approach I'll ask the
appropriate people to update MAINTAINERS.
Second stage MMU:
Defined in the ARMv8 ARM, or the ARMv7 LPAE extensions[*, **].
Second stage SMMU/IOMMU:
Defined in "ARM System Memory Management Unit Architecture
Specification" (ARM IHI 0062D?)
PCI host bridges:
We don't actually (properly) support PCI yet, but see my mails
in the "Enhance platform support for PCI" thread this morning,
for what we think the general shape might be taking.
The meaning of the PCI CFG (CAM, ECAM etc), MMIO and IO (MMIO
mapped) regions are, I think, all pretty well defined by the
(various?) PCI spec(s).
What's not quite so clear cut is the discovery of where the
controllers actually live (based on the fact that
Documentation/devicetree/bindings/pci/ has more than one entry
in it...).
SBSA doesn't really cover this either, it says "The base address
of each ECAM region within the system memory map is
IMPLEMENTATION DEFINED and is expected to be discoverable from
system firmware data."
However I think it will be the case that most "pci host bridge
drivers" for Xen will end up being fairly trivial affairs, i.e.
parse the DT, perhaps a little bit of basic setup and register
the resulting regions with the PCI core and perhaps provide some
simple accessors.
So, I think we can say that for PCI controllers which export a
set of PCI standard CFG, MMIO, IO space regions (all as MMIO
mapped regions) and just require a specific driver for discovery
of the hos
Re: [Xen-devel] [PATCH v5.99.1 RFC 1/4] xen/arm: Duplicate gic-v2.c file to support hip04 platform version
On Wed, 2015-02-25 at 16:34 +, Stefano Stabellini wrote: > I am concerned about the increased size of the Xen binary as a result of > the introduction of this driver. I'm also somewhat concerned about the ongoing maintenance of a proliferation of (subtly or otherwise) different interrupt controllers. (I remember too well the morass which Linux/ARM was in a few years ago with all the h/w variations, which they have spent many years getting on top of.) Speaking only for myself I am (obviously) happy to be part of the maintenance effort for the architecturally defined(/compliant, whatever) ones and of the generic infrastructure. But I think it is reasonable to expect the interested community members to pick up the burden for anything else they would like to add, rather than expecting the core maintainers to do it. (which BTW implies this patch should include a hunk touching MAINTAINERS) I also think it would be reasonable for us to drop support for hardware which is not being adequately maintained. (Not that I expect that to happen here, but it is inevitable that eventually we will see such drivers I think). "adequately maintained" would mean things like timely responses to core infrastructure updates and not bit-rotting. > I think we should disable the build of all drivers in Xen by default, > except for the ARM standard compliant ones (for aarch64 the SBSA is a > nice summary of what is considered compliant), to keep the size of the > binary small. I don't think the SBSA is necessarily the best reference here, since it only defines what is standardised within the scope of "server systems" (whatever you take that to mean) and there are things which do not fall under that umbrella. That said I'm not sure what better reference there is. Maybe even on non-server systems the set of hardware which Xen has to drive itself is limited to things which are covered by the SBSA in practice, by the nature of the fact that the majority of the wacky stuff is driven from hardware domains. So maybe SBSA is OK then... > Could you please introduce a Xen build time option in > xen/arch/arm/Rules.mk, called HAS_NON_STANDARD_DRIVERS, that by default > is n, and gate the build of gic-hip04.c on it? I'm rather wary of creating a "two tier" system like this, but I cannot think of a better compromise :-( Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v5.99.1 RFC 1/4] xen/arm: Duplicate gic-v2.c file to support hip04 platform version
On Wed, 25 Feb 2015, Frediano Ziglio wrote:
> HiSilison Hip04 platform use a slightly different version
>
> Signed-off-by: Frediano Ziglio
I think that this is preferable to the previous approach of modifying
the existing gic-v2 driver, after all the hip04 interrupt controller is
not a gicv2.
> xen/arch/arm/Makefile| 2 +-
> xen/arch/arm/gic-hip04.c | 788
> +++
> 2 files changed, 789 insertions(+), 1 deletion(-)
> create mode 100644 xen/arch/arm/gic-hip04.c
I am concerned about the increased size of the Xen binary as a result of
the introduction of this driver.
I think we should disable the build of all drivers in Xen by default,
except for the ARM standard compliant ones (for aarch64 the SBSA is a
nice summary of what is considered compliant), to keep the size of the
binary small.
Could you please introduce a Xen build time option in
xen/arch/arm/Rules.mk, called HAS_NON_STANDARD_DRIVERS, that by default
is n, and gate the build of gic-hip04.c on it?
> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index 41aba2e..6fb8ba9 100644
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -11,7 +11,7 @@ obj-y += vpsci.o
> obj-y += domctl.o
> obj-y += sysctl.o
> obj-y += domain_build.o
> -obj-y += gic.o gic-v2.o
> +obj-y += gic.o gic-v2.o gic-hip04.o
> obj-$(CONFIG_ARM_64) += gic-v3.o
> obj-y += io.o
> obj-y += irq.o
> diff --git a/xen/arch/arm/gic-hip04.c b/xen/arch/arm/gic-hip04.c
> new file mode 100644
> index 000..a401e3f
> --- /dev/null
> +++ b/xen/arch/arm/gic-hip04.c
> @@ -0,0 +1,788 @@
> +/*
> + * xen/arch/arm/gic-v2.c
gic-hip04.c
> + * ARM Generic Interrupt Controller support v2
This is not an ARM Generic Interrupt Controller v2
> + * Tim Deegan
> + * Copyright (c) 2011 Citrix Systems.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +
> +/*
> + * LR register definitions are GIC v2 specific.
> + * Moved these definitions from header file to here
> + */
> +#define GICH_V2_LR_VIRTUAL_MASK0x3ff
> +#define GICH_V2_LR_VIRTUAL_SHIFT 0
> +#define GICH_V2_LR_PHYSICAL_MASK 0x3ff
> +#define GICH_V2_LR_PHYSICAL_SHIFT 10
> +#define GICH_V2_LR_STATE_MASK 0x3
> +#define GICH_V2_LR_STATE_SHIFT 28
> +#define GICH_V2_LR_PRIORITY_SHIFT 23
> +#define GICH_V2_LR_PRIORITY_MASK 0x1f
> +#define GICH_V2_LR_HW_SHIFT31
> +#define GICH_V2_LR_HW_MASK 0x1
> +#define GICH_V2_LR_GRP_SHIFT 30
> +#define GICH_V2_LR_GRP_MASK0x1
> +#define GICH_V2_LR_MAINTENANCE_IRQ (1<<19)
> +#define GICH_V2_LR_GRP1(1<<30)
> +#define GICH_V2_LR_HW (1<<31)
> +#define GICH_V2_LR_CPUID_SHIFT 9
> +#define GICH_V2_VTR_NRLRGS 0x3f
> +
> +#define GICH_V2_VMCR_PRIORITY_MASK 0x1f
> +#define GICH_V2_VMCR_PRIORITY_SHIFT 27
Please rename all constants, definitions and function names to something
else to make it clear that this driver is not for a gicv2.
Replacing gicv2 with hip04-gic would be a good start.
> +/* Global state */
> +static struct {
> +paddr_t dbase;/* Address of distributor registers */
> +void __iomem * map_dbase; /* IO mapped Address of distributor registers
> */
> +paddr_t cbase;/* Address of CPU interface registers */
> +void __iomem * map_cbase[2]; /* IO mapped Address of CPU interface
> registers */
> +paddr_t hbase;/* Address of virtual interface registers */
> +void __iomem * map_hbase; /* IO Address of virtual interface registers */
> +paddr_t vbase;/* Address of virtual cpu interface registers
> */
> +spinlock_t lock;
> +} gicv2;
> +
> +static struct gic_info gicv2_info;
> +
> +/* The GIC mapping of CPU interfaces does not necessarily match the
> + * logical CPU numbering. Let's use mapping as returned by the GIC
> + * itself
> + */
> +static DEFINE_PER_CPU(u8, gic_cpu_id);
> +
> +/* Maximum cpu interface per GIC */
> +#define NR_GIC_CPU_IF 8
> +
> +static inline void writeb_gicd(uint8_t val, unsigned int offset)
> +{
> +writeb_relaxed(val, gicv2.map_dbase + offset);
> +}
> +
> +static inline void writel_gicd(uint32_t val, unsigned int offset)
> +{
> +writel_relaxed(val, gicv2.map_dbase + offset);
> +}
> +
> +static inline uint32_t readl_gicd(uns
Re: [Xen-devel] [PATCH v5.99.1 RFC 1/4] xen/arm: Duplicate gic-v2.c file to support hip04 platform version
On Wed, 2015-02-25 at 15:34 +, Julien Grall wrote: > Hi Frediano, > > On 25/02/15 15:28, Frediano Ziglio wrote: > > HiSilison Hip04 platform use a slightly different version > > I honestly don't like the idea to copy the whole GIC-v2 drivers. It will > require more maintenance for us. Will it? This puts the hip driver in its own file where I presume Frediano et al will take care of it. > Is there any way to introduce callbacks in the GICv2 code where the > hisilicon hardware differs? That's even worse than the first approach (if (hip()) in gic-v2.c) IMHO. The if-s (or hooks) would be a maintenance burden in the gic-v2.c driver, making maintenance harder for all the other platforms which actually followed the spec. > > > Signed-off-by: Frediano Ziglio > > --- > > xen/arch/arm/Makefile| 2 +- > > xen/arch/arm/gic-hip04.c | 788 > > +++ > > 2 files changed, 789 insertions(+), 1 deletion(-) > > create mode 100644 xen/arch/arm/gic-hip04.c > > > > diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile > > index 41aba2e..6fb8ba9 100644 > > --- a/xen/arch/arm/Makefile > > +++ b/xen/arch/arm/Makefile > > @@ -11,7 +11,7 @@ obj-y += vpsci.o > > obj-y += domctl.o > > obj-y += sysctl.o > > obj-y += domain_build.o > > -obj-y += gic.o gic-v2.o > > +obj-y += gic.o gic-v2.o gic-hip04.o > > As you do a verbatim copy, you should enable to compilation on the next > patch. > > Regards, > ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v5.99.1 RFC 1/4] xen/arm: Duplicate gic-v2.c file to support hip04 platform version
> > Hi Frediano, > > On 25/02/15 15:28, Frediano Ziglio wrote: > > HiSilison Hip04 platform use a slightly different version > > I honestly don't like the idea to copy the whole GIC-v2 drivers. It > will require more maintenance for us. > > Is there any way to introduce callbacks in the GICv2 code where the > hisilicon hardware differs? > > > Signed-off-by: Frediano Ziglio > > --- > > xen/arch/arm/Makefile| 2 +- > > xen/arch/arm/gic-hip04.c | 788 > > +++ > > 2 files changed, 789 insertions(+), 1 deletion(-) create mode > 100644 > > xen/arch/arm/gic-hip04.c > > > > diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile index > > 41aba2e..6fb8ba9 100644 > > --- a/xen/arch/arm/Makefile > > +++ b/xen/arch/arm/Makefile > > @@ -11,7 +11,7 @@ obj-y += vpsci.o > > obj-y += domctl.o > > obj-y += sysctl.o > > obj-y += domain_build.o > > -obj-y += gic.o gic-v2.o > > +obj-y += gic.o gic-v2.o gic-hip04.o > > As you do a verbatim copy, you should enable to compilation on the next > patch. > I understood now what you where saying. Yes, you are right, single patch should just contains the copy Frediano ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v5.99.1 RFC 1/4] xen/arm: Duplicate gic-v2.c file to support hip04 platform version
> > Hi Frediano, > > On 25/02/15 15:28, Frediano Ziglio wrote: > > HiSilison Hip04 platform use a slightly different version > > I honestly don't like the idea to copy the whole GIC-v2 drivers. It > will require more maintenance for us. > > Is there any way to introduce callbacks in the GICv2 code where the > hisilicon hardware differs? > This is why I introduced RFC back. I proposed two technique, one is copy (this), the other is templating. I'll send the other implementation (once done). If I understood function callback (that require quite a lot of changes and slow down execution) was not an option. > > Signed-off-by: Frediano Ziglio > > --- > > xen/arch/arm/Makefile| 2 +- > > xen/arch/arm/gic-hip04.c | 788 > > +++ > > 2 files changed, 789 insertions(+), 1 deletion(-) create mode > 100644 > > xen/arch/arm/gic-hip04.c > > > > diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile index > > 41aba2e..6fb8ba9 100644 > > --- a/xen/arch/arm/Makefile > > +++ b/xen/arch/arm/Makefile > > @@ -11,7 +11,7 @@ obj-y += vpsci.o > > obj-y += domctl.o > > obj-y += sysctl.o > > obj-y += domain_build.o > > -obj-y += gic.o gic-v2.o > > +obj-y += gic.o gic-v2.o gic-hip04.o > > As you do a verbatim copy, you should enable to compilation on the next > patch. > No, this is done on purpose as git does not support copy. This make easier to understand the changes from the base which now is present as single commit. Frediano > Regards, > > -- > Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v5.99.1 RFC 1/4] xen/arm: Duplicate gic-v2.c file to support hip04 platform version
Hi Frediano, On 25/02/15 15:28, Frediano Ziglio wrote: > HiSilison Hip04 platform use a slightly different version I honestly don't like the idea to copy the whole GIC-v2 drivers. It will require more maintenance for us. Is there any way to introduce callbacks in the GICv2 code where the hisilicon hardware differs? > Signed-off-by: Frediano Ziglio > --- > xen/arch/arm/Makefile| 2 +- > xen/arch/arm/gic-hip04.c | 788 > +++ > 2 files changed, 789 insertions(+), 1 deletion(-) > create mode 100644 xen/arch/arm/gic-hip04.c > > diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile > index 41aba2e..6fb8ba9 100644 > --- a/xen/arch/arm/Makefile > +++ b/xen/arch/arm/Makefile > @@ -11,7 +11,7 @@ obj-y += vpsci.o > obj-y += domctl.o > obj-y += sysctl.o > obj-y += domain_build.o > -obj-y += gic.o gic-v2.o > +obj-y += gic.o gic-v2.o gic-hip04.o As you do a verbatim copy, you should enable to compilation on the next patch. Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v5.99.1 RFC 1/4] xen/arm: Duplicate gic-v2.c file to support hip04 platform version
HiSilison Hip04 platform use a slightly different version
Signed-off-by: Frediano Ziglio
---
xen/arch/arm/Makefile| 2 +-
xen/arch/arm/gic-hip04.c | 788 +++
2 files changed, 789 insertions(+), 1 deletion(-)
create mode 100644 xen/arch/arm/gic-hip04.c
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 41aba2e..6fb8ba9 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -11,7 +11,7 @@ obj-y += vpsci.o
obj-y += domctl.o
obj-y += sysctl.o
obj-y += domain_build.o
-obj-y += gic.o gic-v2.o
+obj-y += gic.o gic-v2.o gic-hip04.o
obj-$(CONFIG_ARM_64) += gic-v3.o
obj-y += io.o
obj-y += irq.o
diff --git a/xen/arch/arm/gic-hip04.c b/xen/arch/arm/gic-hip04.c
new file mode 100644
index 000..a401e3f
--- /dev/null
+++ b/xen/arch/arm/gic-hip04.c
@@ -0,0 +1,788 @@
+/*
+ * xen/arch/arm/gic-v2.c
+ *
+ * ARM Generic Interrupt Controller support v2
+ *
+ * Tim Deegan
+ * Copyright (c) 2011 Citrix Systems.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+
+/*
+ * LR register definitions are GIC v2 specific.
+ * Moved these definitions from header file to here
+ */
+#define GICH_V2_LR_VIRTUAL_MASK0x3ff
+#define GICH_V2_LR_VIRTUAL_SHIFT 0
+#define GICH_V2_LR_PHYSICAL_MASK 0x3ff
+#define GICH_V2_LR_PHYSICAL_SHIFT 10
+#define GICH_V2_LR_STATE_MASK 0x3
+#define GICH_V2_LR_STATE_SHIFT 28
+#define GICH_V2_LR_PRIORITY_SHIFT 23
+#define GICH_V2_LR_PRIORITY_MASK 0x1f
+#define GICH_V2_LR_HW_SHIFT31
+#define GICH_V2_LR_HW_MASK 0x1
+#define GICH_V2_LR_GRP_SHIFT 30
+#define GICH_V2_LR_GRP_MASK0x1
+#define GICH_V2_LR_MAINTENANCE_IRQ (1<<19)
+#define GICH_V2_LR_GRP1(1<<30)
+#define GICH_V2_LR_HW (1<<31)
+#define GICH_V2_LR_CPUID_SHIFT 9
+#define GICH_V2_VTR_NRLRGS 0x3f
+
+#define GICH_V2_VMCR_PRIORITY_MASK 0x1f
+#define GICH_V2_VMCR_PRIORITY_SHIFT 27
+
+/* Global state */
+static struct {
+paddr_t dbase;/* Address of distributor registers */
+void __iomem * map_dbase; /* IO mapped Address of distributor registers */
+paddr_t cbase;/* Address of CPU interface registers */
+void __iomem * map_cbase[2]; /* IO mapped Address of CPU interface
registers */
+paddr_t hbase;/* Address of virtual interface registers */
+void __iomem * map_hbase; /* IO Address of virtual interface registers */
+paddr_t vbase;/* Address of virtual cpu interface registers */
+spinlock_t lock;
+} gicv2;
+
+static struct gic_info gicv2_info;
+
+/* The GIC mapping of CPU interfaces does not necessarily match the
+ * logical CPU numbering. Let's use mapping as returned by the GIC
+ * itself
+ */
+static DEFINE_PER_CPU(u8, gic_cpu_id);
+
+/* Maximum cpu interface per GIC */
+#define NR_GIC_CPU_IF 8
+
+static inline void writeb_gicd(uint8_t val, unsigned int offset)
+{
+writeb_relaxed(val, gicv2.map_dbase + offset);
+}
+
+static inline void writel_gicd(uint32_t val, unsigned int offset)
+{
+writel_relaxed(val, gicv2.map_dbase + offset);
+}
+
+static inline uint32_t readl_gicd(unsigned int offset)
+{
+return readl_relaxed(gicv2.map_dbase + offset);
+}
+
+static inline void writel_gicc(uint32_t val, unsigned int offset)
+{
+unsigned int page = offset >> PAGE_SHIFT;
+offset &= ~PAGE_MASK;
+writel_relaxed(val, gicv2.map_cbase[page] + offset);
+}
+
+static inline uint32_t readl_gicc(unsigned int offset)
+{
+unsigned int page = offset >> PAGE_SHIFT;
+offset &= ~PAGE_MASK;
+return readl_relaxed(gicv2.map_cbase[page] + offset);
+}
+
+static inline void writel_gich(uint32_t val, unsigned int offset)
+{
+writel_relaxed(val, gicv2.map_hbase + offset);
+}
+
+static inline uint32_t readl_gich(int unsigned offset)
+{
+return readl_relaxed(gicv2.map_hbase + offset);
+}
+
+static unsigned int gicv2_cpu_mask(const cpumask_t *cpumask)
+{
+unsigned int cpu;
+unsigned int mask = 0;
+cpumask_t possible_mask;
+
+cpumask_and(&possible_mask, cpumask, &cpu_possible_map);
+for_each_cpu( cpu, &possible_mask )
+{
+ASSERT(cpu < NR_GIC_CPU_IF);
+mask |= per_cpu(gic_cpu_id, cpu);
+}
+
+return mask;
+}
+
+static void gicv2_save_state(struct vcpu *v)
+{
+int i;
+
+/* No need for spinlocks here because int
