* RE: [PATCH 1/1] Add a new platform tree for ib8315. Also add the DTS and the defconfig for that board.
From: Sergey Gerasimov @ 2013-03-19 10:09 UTC (permalink / raw)
To: Kumar Gala
Cc: Paul Mackerras, linuxppc-dev@lists.ozlabs.org,
linux-kernel@vger.kernel.org
In-Reply-To: <307780DE-9735-4329-B5D7-DF2E9451A217@kernel.crashing.org>
Yes. Ib8315 based on tqm8315. Ib8315 is baseboard that has the tqm8215m plu=
gged in as "the central part".=20
-----Original Message-----
From: Kumar Gala [mailto:galak@kernel.crashing.org]=20
Sent: 19 =CD=C1=D2=D4=C1 2013 =C7. 00:18
To: Sergey Gerasimov
Cc: Benjamin Herrenschmidt; Paul Mackerras; linuxppc-dev@lists.ozlabs.org; =
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/1] Add a new platform tree for ib8315. Also add the D=
TS and the defconfig for that board.
On Mar 18, 2013, at 8:47 AM, Sergey Gerasimov wrote:
> Signed-off-by: Sergey Gerasimov=20
> <Sergey.Gerasimov@astrosoft-development.com>
> ---
> arch/powerpc/boot/dts/ib8315.dts | 490 +++++++
> arch/powerpc/configs/83xx/ib8315_defconfig | 2121 +++++++++++++++++++++++=
+++++
> arch/powerpc/platforms/83xx/Kconfig | 7 +
> arch/powerpc/platforms/83xx/Makefile | 1 +
> arch/powerpc/platforms/83xx/tqm8315.c | 137 ++
> 5 files changed, 2756 insertions(+)
> create mode 100644 arch/powerpc/boot/dts/ib8315.dts create mode 100644=20
> arch/powerpc/configs/83xx/ib8315_defconfig
> create mode 100644 arch/powerpc/platforms/83xx/tqm8315.c
Is there some relationship of this board with the TQM8315?
- k
^ permalink raw reply
* using request_irq_percpu()
From: Bhushan Bharat-R65777 @ 2013-03-19 14:55 UTC (permalink / raw)
To: linuxppc-dev@lists.ozlabs.org, Wood Scott-B07421
Hi All,
request_irq_percpu() is defined in kernel/irq/manage.c, this takes a percpu=
pointer which will be unique based upon on which cpu the handler executes.
So, it looks like we can use this to have multiple bottom half interrupt ha=
ndler executing at same time on different CPU and each can handle this inde=
pendently.
Flow will be like:
-- Interrupt occurs on CPU1 - handler save some context for bottom half and=
then clears the interrupt condition, and return (in between the interrupt =
affinity will be moved to next CPU in round robin fashion).
-- CPU 1 executing its bottom half.
-- Again interrupt occurs, which will come on CPU 2
-- CPU 2 handler similar to CPU1 and so on.
This way multiple similar bottom half can run at same time on different CPU
Thanks
-Bharat
^ permalink raw reply
* Re: [PATCH][UPSTEAM] powerpc/mpic: add irq_set_wake support
From: Scott Wood @ 2013-03-19 16:00 UTC (permalink / raw)
To: Wang Dongsheng-B40534; +Cc: Wood Scott-B07421, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <ABB05CD9C9F68C46A5CEDC7F15439259EB715D@039-SN2MPN1-022.039d.mgd.msft.net>
On 03/19/2013 02:10:38 AM, Wang Dongsheng-B40534 wrote:
>=20
>=20
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Tuesday, March 19, 2013 7:36 AM
> > To: Wang Dongsheng-B40534
> > Cc: linuxppc-dev@lists.ozlabs.org; galak@kernel.crashing.org
> > Subject: Re: [PATCH][UPSTEAM] powerpc/mpic: add irq_set_wake support
> >
> > On 01/30/2013 09:10:23 PM, Wang Dongsheng wrote:
> > > Add irq_set_wake support. Just add IRQF_NO_SUSPEND to
> > > desc->action->flag.
> > > So the wake up interrupt will not be disable in =20
> suspend_device_irqs.
> > >
> > > Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>
> > > ---
> > > arch/powerpc/sysdev/mpic.c | 15 +++++++++++++++
> > > 1 files changed, 15 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/arch/powerpc/sysdev/mpic.c =20
> b/arch/powerpc/sysdev/mpic.c
> > > index 9c6e535..2ed0220 100644
> > > --- a/arch/powerpc/sysdev/mpic.c
> > > +++ b/arch/powerpc/sysdev/mpic.c
> > > @@ -920,6 +920,18 @@ int mpic_set_irq_type(struct irq_data *d,
> > > unsigned int flow_type)
> > > return IRQ_SET_MASK_OK_NOCOPY;
> > > }
> > >
> > > +static int mpic_irq_set_wake(struct irq_data *d, unsigned int on)
> > > +{
> > > + struct irq_desc *desc =3D container_of(d, struct irq_desc,
> > > irq_data);
> > > +
> > > + if (on)
> > > + desc->action->flags |=3D IRQF_NO_SUSPEND;
> > > + else
> > > + desc->action->flags &=3D ~IRQF_NO_SUSPEND;
> > > +
> > > + return 0;
> > > +}
> >
> > This should really be something like fsl_mpic_irq_set_wake() and =20
> only
> > set when we have an FSL MPIC.
> >
> Thanks, Add "#ifdef FSL_SOC" to control.
That's not what I meant. Check "mpic->flags & MPIC_FSL".
-Scott=
^ permalink raw reply
* Re: [PATCH V2] powerpc/85xx: workaround for chips with MSI hardware errata
From: Scott Wood @ 2013-03-19 16:31 UTC (permalink / raw)
To: Jia Hongtao-B38951
Cc: Wood Scott-B07421, linuxppc-dev@lists.ozlabs.org, Li Yang-R58472
In-Reply-To: <412C8208B4A0464FA894C5F0C278CD5D01C1AB0B@039-SN1MPN1-003.039d.mgd.msft.net>
;On 03/19/2013 03:03:13 AM, Jia Hongtao-B38951 wrote:
>=20
>=20
> > -----Original Message-----
> > From: Kumar Gala [mailto:galak@kernel.crashing.org]
> > Sent: Friday, March 15, 2013 11:53 PM
> > To: Jia Hongtao-B38951
> > Cc: linuxppc-dev@lists.ozlabs.org; Wood Scott-B07421;
> > michael@ellerman.id.au; Li Yang-R58472
> > Subject: Re: [PATCH V2] powerpc/85xx: workaround for chips with MSI
> > hardware errata
> >
> >
> > On Mar 14, 2013, at 9:00 PM, Jia Hongtao-B38951 wrote:
> >
> > >
> > >
> > >> -----Original Message-----
> > >> From: Kumar Gala [mailto:galak@kernel.crashing.org]
> > >> Sent: Friday, March 15, 2013 4:05 AM
> > >> To: Jia Hongtao-B38951
> > >> Cc: linuxppc-dev@lists.ozlabs.org; Wood Scott-B07421;
> > >> michael@ellerman.id.au; Li Yang-R58472; Jia Hongtao-B38951
> > >> Subject: Re: [PATCH V2] powerpc/85xx: workaround for chips with =20
> MSI
> > >> hardware errata
> > >>
> > >>
> > >> On Mar 14, 2013, at 5:35 AM, Jia Hongtao wrote:
> > >>
> > >>> The MPIC version 2.0 has a MSI errata (errata PIC1 of mpc8544), =20
> It
> > >>> causes that neither MSI nor MSI-X can work fine. This is a
> > >>> workaround to allow MSI-X to function properly.
> > >>>
> > >>> Signed-off-by: Liu Shuo <soniccat.liu@gmail.com>
> > >>> Signed-off-by: Li Yang <leoli@freescale.com>
> > >>> Signed-off-by: Jia Hongtao <hongtao.jia@freescale.com>
> > >>> ---
> > >>> Changes for V2:
> > >>> - Address almost all the comments from Michael Ellerman for V1.
> > >>> Here is the link:
> > >>> http://patchwork.ozlabs.org/patch/226833/
> > >>>
> > >>> arch/powerpc/sysdev/fsl_msi.c | 65
> > >>> +++++++++++++++++++++++++++++++++++++++++--
> > >>> arch/powerpc/sysdev/fsl_msi.h | 2 ++
> > >>> 2 files changed, 64 insertions(+), 3 deletions(-)
> > >>>
> > >>> diff --git a/arch/powerpc/sysdev/fsl_msi.c
> > >>> b/arch/powerpc/sysdev/fsl_msi.c index 178c994..54cb83e 100644
> > >>> --- a/arch/powerpc/sysdev/fsl_msi.c
> > >>> +++ b/arch/powerpc/sysdev/fsl_msi.c
> > >>> @@ -98,8 +98,18 @@ static int fsl_msi_init_allocator(struct =20
> fsl_msi
> > >>> *msi_data)
> > >>>
> > >>> static int fsl_msi_check_device(struct pci_dev *pdev, int nvec, =20
> int
> > >>> type) {
> > >>> - if (type =3D=3D PCI_CAP_ID_MSIX)
> > >>> - pr_debug("fslmsi: MSI-X untested, trying =20
> anyway.\n");
> > >>> + struct fsl_msi *msi;
> > >>> +
> > >>> + if (type =3D=3D PCI_CAP_ID_MSI) {
> > >>> + /*
> > >>> + * MPIC version 2.0 has erratum PIC1. For now =20
> MSI
> > >>> + * could not work. So check to prevent MSI from
> > >>> + * being used on the board with this erratum.
> > >>> + */
> > >>> + list_for_each_entry(msi, &msi_head, list)
> > >>> + if (msi->feature & MSI_HW_ERRATA_ENDIAN)
> > >>> + return -EINVAL;
> > >>> + }
> > >>>
> > >>> return 0;
> > >>> }
> > >>> @@ -142,7 +152,17 @@ static void fsl_compose_msi_msg(struct =20
> pci_dev
> > >> *pdev, int hwirq,
> > >>> msg->address_lo =3D lower_32_bits(address);
> > >>> msg->address_hi =3D upper_32_bits(address);
> > >>>
> > >>> - msg->data =3D hwirq;
> > >>> + /*
> > >>> + * MPIC version 2.0 has erratum PIC1. It causes
> > >>> + * that neither MSI nor MSI-X can work fine.
> > >>> + * This is a workaround to allow MSI-X to function
> > >>> + * properly. It only works for MSI-X, we prevent
> > >>> + * MSI on buggy chips in fsl_msi_check_device().
> > >>> + */
> > >>> + if (msi_data->feature & MSI_HW_ERRATA_ENDIAN)
> > >>> + msg->data =3D __swab32(hwirq);
> > >>> + else
> > >>> + msg->data =3D hwirq;
> > >>>
> > >>> pr_debug("%s: allocated srs: %d, ibs: %d\n",
> > >>> __func__, hwirq / IRQS_PER_MSI_REG, hwirq % =20
> IRQS_PER_MSI_REG);
> > >> @@
> > >>> -361,6 +381,35 @@ static int fsl_msi_setup_hwirq(struct fsl_msi
> > >>> *msi,
> > >> struct platform_device *dev,
> > >>> return 0;
> > >>> }
> > >>>
> > >>> +/* MPIC version 2.0 has erratum PIC1 */ static int
> > >>> +mpic_has_errata(struct platform_device *dev) {
> > >>> + struct device_node *mpic_node;
> > >>> +
> > >>> + mpic_node =3D of_irq_find_parent(dev->dev.of_node);
> > >>> + if (mpic_node) {
> > >>> + u32 *reg_base, brr1 =3D 0;
> > >>> + /* Get the PIC reg base */
> > >>> + reg_base =3D of_iomap(mpic_node, 0);
> > >>> + of_node_put(mpic_node);
> > >>> + if (!reg_base) {
> > >>> + dev_err(&dev->dev, "ioremap problem =20
> failed.\n");
> > >>> + return -EIO;
> > >>> + }
> > >>> +
> > >>> + /* Get the mpic version from block revision =20
> register 1 */
> > >>> + brr1 =3D in_be32(reg_base + MPIC_FSL_BRR1);
> > >>> + iounmap(reg_base);
> > >>> + if ((brr1 & MPIC_FSL_BRR1_VER) =3D=3D 0x0200)
> > >>> + return 1;
> > >>> + } else {
> > >>> + dev_err(&dev->dev, "MSI can't find his parent =20
> mpic node.\n");
> > >>> + return -ENODEV;
> > >>> + }
> > >>> +
> > >>> + return 0;
> > >>> +}
> > >>> +
> > >>> static const struct of_device_id fsl_of_msi_ids[]; static int
> > >>> fsl_of_msi_probe(struct platform_device *dev) { @@ -423,6 =20
> +472,16 @@
> > >>> static int fsl_of_msi_probe(struct platform_device *dev)
> > >>>
> > >>> msi->feature =3D features->fsl_pic_ip;
> > >>>
> > >>> + if ((features->fsl_pic_ip & FSL_PIC_IP_MASK) =3D=3D =20
> FSL_PIC_IP_MPIC) {
> > >>> + rc =3D mpic_has_errata(dev);
> > >>> + if (rc > 0) {
> > >>> + msi->feature |=3D MSI_HW_ERRATA_ENDIAN;
> > >>> + } else if (rc < 0) {
> > >>> + err =3D rc;
> > >>> + goto error_out;
> > >>> + }
> > >>> + }
> > >>> +
> > >>> /*
> > >>> * Remember the phandle, so that we can match with any =20
> PCI nodes
> > >>> * that have an "fsl,msi" property.
> > >>> diff --git a/arch/powerpc/sysdev/fsl_msi.h
> > >>> b/arch/powerpc/sysdev/fsl_msi.h index 8225f86..7389e8e 100644
> > >>> --- a/arch/powerpc/sysdev/fsl_msi.h
> > >>> +++ b/arch/powerpc/sysdev/fsl_msi.h
> > >>> @@ -25,6 +25,8 @@
> > >>> #define FSL_PIC_IP_IPIC 0x00000002
> > >>> #define FSL_PIC_IP_VMPIC 0x00000003
> > >>>
> > >>> +#define MSI_HW_ERRATA_ENDIAN 0x00000010
> > >>> +
> > >>
> > >> Is there any reason to put this in fsl_msi.h rather than just in
> > >> fsl_msi.c itself?
> > >>
> > >> - k
> > >
> > > Actually no. This micro is only used by fsl_msi.c.
> > > Will move it to fsl_msi.c.
> > >
> > > Thanks.
> > > -Hongtao.
> >
> > Also, wondering if we can do the mpic version detection in mpic.c =20
> and not
> > here. I'm not sure what means we'd have to get back to the mpic =20
> struct
> > so we could possible use mpic->flags.
> >
>=20
> Use the MPIC version result in mpic.c was my plan.
> But as you point out there seems no obvious way to get the mpic =20
> struct.
> mpic struct is defined as an automatic variable in platform files.
> Also MSI driver is not so close to mpic driver under current =20
> architecture.
>=20
> If you get some elegant way to do this please feel free to tell me.
Declare a non-static function to retrieve the MPIC version.
-Scott=
^ permalink raw reply
* [PATCH] KVM: PPC: e500: Add separate functions for vcpu's MMU configuration
From: Mihai Caraman @ 2013-03-19 17:16 UTC (permalink / raw)
To: kvm-ppc; +Cc: Mihai Caraman, linuxppc-dev, kvm
Move vcpu's MMU default configuration and geometry update into their own
functions.
Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
---
arch/powerpc/kvm/e500_mmu.c | 59 +++++++++++++++++++++++++++----------------
1 files changed, 37 insertions(+), 22 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu.c b/arch/powerpc/kvm/e500_mmu.c
index 5c44759..66b6e31 100644
--- a/arch/powerpc/kvm/e500_mmu.c
+++ b/arch/powerpc/kvm/e500_mmu.c
@@ -596,6 +596,20 @@ int kvmppc_set_sregs_e500_tlb(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs)
return 0;
}
+static int vcpu_mmu_geometry_update(struct kvm_vcpu *vcpu,
+ struct kvm_book3e_206_tlb_params *params)
+{
+ vcpu->arch.tlbcfg[0] &= ~(TLBnCFG_N_ENTRY | TLBnCFG_ASSOC);
+ if (params->tlb_sizes[0] <= 2048)
+ vcpu->arch.tlbcfg[0] |= params->tlb_sizes[0];
+ vcpu->arch.tlbcfg[0] |= params->tlb_ways[0] << TLBnCFG_ASSOC_SHIFT;
+
+ vcpu->arch.tlbcfg[1] &= ~(TLBnCFG_N_ENTRY | TLBnCFG_ASSOC);
+ vcpu->arch.tlbcfg[1] |= params->tlb_sizes[1];
+ vcpu->arch.tlbcfg[1] |= params->tlb_ways[1] << TLBnCFG_ASSOC_SHIFT;
+ return 0;
+}
+
int kvm_vcpu_ioctl_config_tlb(struct kvm_vcpu *vcpu,
struct kvm_config_tlb *cfg)
{
@@ -692,16 +706,8 @@ int kvm_vcpu_ioctl_config_tlb(struct kvm_vcpu *vcpu,
vcpu_e500->gtlb_offset[0] = 0;
vcpu_e500->gtlb_offset[1] = params.tlb_sizes[0];
- vcpu->arch.mmucfg = mfspr(SPRN_MMUCFG) & ~MMUCFG_LPIDSIZE;
-
- vcpu->arch.tlbcfg[0] &= ~(TLBnCFG_N_ENTRY | TLBnCFG_ASSOC);
- if (params.tlb_sizes[0] <= 2048)
- vcpu->arch.tlbcfg[0] |= params.tlb_sizes[0];
- vcpu->arch.tlbcfg[0] |= params.tlb_ways[0] << TLBnCFG_ASSOC_SHIFT;
-
- vcpu->arch.tlbcfg[1] &= ~(TLBnCFG_N_ENTRY | TLBnCFG_ASSOC);
- vcpu->arch.tlbcfg[1] |= params.tlb_sizes[1];
- vcpu->arch.tlbcfg[1] |= params.tlb_ways[1] << TLBnCFG_ASSOC_SHIFT;
+ /* Update vcpu's MMU geometry based on SW_TLB input */
+ vcpu_mmu_geometry_update(vcpu, ¶ms);
vcpu_e500->shared_tlb_pages = pages;
vcpu_e500->num_shared_tlb_pages = num_pages;
@@ -737,6 +743,26 @@ int kvm_vcpu_ioctl_dirty_tlb(struct kvm_vcpu *vcpu,
return 0;
}
+/* vcpu's MMU default configuration */
+static int vcpu_mmu_init(struct kvm_vcpu *vcpu,
+ struct kvmppc_e500_tlb_params *params)
+{
+ /* Initialize RASIZE, PIDSIZE, NTLBS and MAVN fields with host values*/
+ vcpu->arch.mmucfg = mfspr(SPRN_MMUCFG) & ~MMUCFG_LPIDSIZE;
+
+ /* Initialize IPROT field with host value*/
+ vcpu->arch.tlbcfg[0] = mfspr(SPRN_TLB0CFG) &
+ ~(TLBnCFG_N_ENTRY | TLBnCFG_ASSOC);
+ vcpu->arch.tlbcfg[0] |= params[0].entries;
+ vcpu->arch.tlbcfg[0] |= params[0].ways << TLBnCFG_ASSOC_SHIFT;
+
+ vcpu->arch.tlbcfg[1] = mfspr(SPRN_TLB1CFG) &
+ ~(TLBnCFG_N_ENTRY | TLBnCFG_ASSOC);
+ vcpu->arch.tlbcfg[1] |= params[1].entries;
+ vcpu->arch.tlbcfg[1] |= params[1].ways << TLBnCFG_ASSOC_SHIFT;
+ return 0;
+}
+
int kvmppc_e500_tlb_init(struct kvmppc_vcpu_e500 *vcpu_e500)
{
struct kvm_vcpu *vcpu = &vcpu_e500->vcpu;
@@ -781,18 +807,7 @@ int kvmppc_e500_tlb_init(struct kvmppc_vcpu_e500 *vcpu_e500)
if (!vcpu_e500->g2h_tlb1_map)
goto err;
- /* Init TLB configuration register */
- vcpu->arch.tlbcfg[0] = mfspr(SPRN_TLB0CFG) &
- ~(TLBnCFG_N_ENTRY | TLBnCFG_ASSOC);
- vcpu->arch.tlbcfg[0] |= vcpu_e500->gtlb_params[0].entries;
- vcpu->arch.tlbcfg[0] |=
- vcpu_e500->gtlb_params[0].ways << TLBnCFG_ASSOC_SHIFT;
-
- vcpu->arch.tlbcfg[1] = mfspr(SPRN_TLB1CFG) &
- ~(TLBnCFG_N_ENTRY | TLBnCFG_ASSOC);
- vcpu->arch.tlbcfg[1] |= vcpu_e500->gtlb_params[1].entries;
- vcpu->arch.tlbcfg[1] |=
- vcpu_e500->gtlb_params[1].ways << TLBnCFG_ASSOC_SHIFT;
+ vcpu_mmu_init(vcpu, vcpu_e500->gtlb_params);
kvmppc_recalc_tlb1map_range(vcpu_e500);
return 0;
--
1.7.4.1
^ permalink raw reply related
* [PATCH] KVM: PPC: e500: Expose MMU registers via ONE_REG
From: Mihai Caraman @ 2013-03-19 17:17 UTC (permalink / raw)
To: kvm-ppc; +Cc: Mihai Caraman, linuxppc-dev, kvm
MMU registers were exposed to user-space using sregs interface. Add them to
ONE_REG interface.
Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
---
Documentation/virtual/kvm/api.txt | 13 +++++
arch/powerpc/include/uapi/asm/kvm.h | 14 ++++++
arch/powerpc/kvm/44x.c | 12 +++++
arch/powerpc/kvm/booke.c | 67 +++++++++++++++++---------
arch/powerpc/kvm/e500.c | 14 ++++++
arch/powerpc/kvm/e500.h | 4 ++
arch/powerpc/kvm/e500_mmu.c | 89 +++++++++++++++++++++++++++++++++++
arch/powerpc/kvm/e500mc.c | 15 ++++++
8 files changed, 204 insertions(+), 24 deletions(-)
diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index c2534c3..9aee4dd 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -1763,6 +1763,19 @@ registers, find a list below:
PPC | KVM_REG_PPC_EPCR | 32
PPC | KVM_REG_PPC_EPR | 32
+ PPC | KVM_REG_PPC_MAS0 | 32
+ PPC | KVM_REG_PPC_MAS1 | 32
+ PPC | KVM_REG_PPC_MAS2 | 64
+ PPC | KVM_REG_PPC_MAS7_3 | 64
+ PPC | KVM_REG_PPC_MAS4 | 32
+ PPC | KVM_REG_PPC_MAS6 | 32
+
+ PPC | KVM_REG_PPC_MMUCFG | 32
+ PPC | KVM_REG_PPC_TLB0CFG | 32
+ PPC | KVM_REG_PPC_TLB1CFG | 32
+ PPC | KVM_REG_PPC_TLB2CFG | 32
+ PPC | KVM_REG_PPC_TLB3CFG | 32
+
4.69 KVM_GET_ONE_REG
Capability: KVM_CAP_ONE_REG
diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
index 16064d0..70dfeef 100644
--- a/arch/powerpc/include/uapi/asm/kvm.h
+++ b/arch/powerpc/include/uapi/asm/kvm.h
@@ -417,4 +417,18 @@ struct kvm_get_htab_header {
#define KVM_REG_PPC_EPCR (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x85)
#define KVM_REG_PPC_EPR (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x86)
+/* MMU registers */
+#define KVM_REG_PPC_MAS0 (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x87)
+#define KVM_REG_PPC_MAS1 (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x88)
+#define KVM_REG_PPC_MAS2 (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x89)
+#define KVM_REG_PPC_MAS7_3 (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x8a)
+#define KVM_REG_PPC_MAS4 (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x8b)
+#define KVM_REG_PPC_MAS6 (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x8c)
+#define KVM_REG_PPC_MMUCFG (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x8d)
+/* TLBnCFG_N_ENTRY and TLBnCFG_ASSOC can be changed only using SW_TLB ioctl */
+#define KVM_REG_PPC_TLB0CFG (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x8e)
+#define KVM_REG_PPC_TLB1CFG (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x8f)
+#define KVM_REG_PPC_TLB2CFG (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x90)
+#define KVM_REG_PPC_TLB3CFG (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x91)
+
#endif /* __LINUX_KVM_POWERPC_H */
diff --git a/arch/powerpc/kvm/44x.c b/arch/powerpc/kvm/44x.c
index 3d7fd21..2f5c6b6 100644
--- a/arch/powerpc/kvm/44x.c
+++ b/arch/powerpc/kvm/44x.c
@@ -124,6 +124,18 @@ int kvmppc_core_set_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs)
return kvmppc_set_sregs_ivor(vcpu, sregs);
}
+int kvmppc_get_one_reg(struct kvm_vcpu *vcpu, u64 id,
+ union kvmppc_one_reg *val)
+{
+ return -EINVAL;
+}
+
+int kvmppc_set_one_reg(struct kvm_vcpu *vcpu, u64 id,
+ union kvmppc_one_reg *val)
+{
+ return -EINVAL;
+}
+
struct kvm_vcpu *kvmppc_core_vcpu_create(struct kvm *kvm, unsigned int id)
{
struct kvmppc_vcpu_44x *vcpu_44x;
diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index 020923e..1b25923d 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -1409,81 +1409,100 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg)
{
- int r = -EINVAL;
+ int r = 0;
+ union kvmppc_one_reg val;
+ int size;
+ long int i;
+
+ size = one_reg_size(reg->id);
+ if (size > sizeof(val))
+ return -EINVAL;
switch (reg->id) {
case KVM_REG_PPC_IAC1:
case KVM_REG_PPC_IAC2:
case KVM_REG_PPC_IAC3:
case KVM_REG_PPC_IAC4: {
- int iac = reg->id - KVM_REG_PPC_IAC1;
- r = copy_to_user((u64 __user *)(long)reg->addr,
- &vcpu->arch.dbg_reg.iac[iac], sizeof(u64));
+ i = reg->id - KVM_REG_PPC_IAC1;
+ val = get_reg_val(reg->id, vcpu->arch.dbg_reg.iac[i]);
break;
}
case KVM_REG_PPC_DAC1:
case KVM_REG_PPC_DAC2: {
- int dac = reg->id - KVM_REG_PPC_DAC1;
- r = copy_to_user((u64 __user *)(long)reg->addr,
- &vcpu->arch.dbg_reg.dac[dac], sizeof(u64));
+ i = reg->id - KVM_REG_PPC_DAC1;
+ val = get_reg_val(reg->id, vcpu->arch.dbg_reg.dac[i]);
break;
}
case KVM_REG_PPC_EPR: {
u32 epr = get_guest_epr(vcpu);
- r = put_user(epr, (u32 __user *)(long)reg->addr);
+ val = get_reg_val(reg->id, epr);
break;
}
#if defined(CONFIG_64BIT)
case KVM_REG_PPC_EPCR:
- r = put_user(vcpu->arch.epcr, (u32 __user *)(long)reg->addr);
+ val = get_reg_val(reg->id, vcpu->arch.epcr);
break;
#endif
default:
+ r = kvmppc_get_one_reg(vcpu, reg->id, &val);
break;
}
+
+ if (r)
+ return r;
+
+ if (copy_to_user((char __user *)(unsigned long)reg->addr, &val, size))
+ r = -EFAULT;
+
return r;
}
int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg)
{
- int r = -EINVAL;
+ int r = 0;
+ union kvmppc_one_reg val;
+ int size;
+ long int i;
+
+ size = one_reg_size(reg->id);
+ if (size > sizeof(val))
+ return -EINVAL;
+
+ if (copy_from_user(&val, (char __user *)(unsigned long)reg->addr, size))
+ return -EFAULT;
switch (reg->id) {
case KVM_REG_PPC_IAC1:
case KVM_REG_PPC_IAC2:
case KVM_REG_PPC_IAC3:
case KVM_REG_PPC_IAC4: {
- int iac = reg->id - KVM_REG_PPC_IAC1;
- r = copy_from_user(&vcpu->arch.dbg_reg.iac[iac],
- (u64 __user *)(long)reg->addr, sizeof(u64));
+ i = reg->id - KVM_REG_PPC_IAC1;
+ vcpu->arch.dbg_reg.iac[i] = set_reg_val(reg->id, val);
break;
}
case KVM_REG_PPC_DAC1:
case KVM_REG_PPC_DAC2: {
- int dac = reg->id - KVM_REG_PPC_DAC1;
- r = copy_from_user(&vcpu->arch.dbg_reg.dac[dac],
- (u64 __user *)(long)reg->addr, sizeof(u64));
+ i = reg->id - KVM_REG_PPC_DAC1;
+ vcpu->arch.dbg_reg.dac[i] = set_reg_val(reg->id, val);
break;
}
case KVM_REG_PPC_EPR: {
- u32 new_epr;
- r = get_user(new_epr, (u32 __user *)(long)reg->addr);
- if (!r)
- kvmppc_set_epr(vcpu, new_epr);
+ u32 new_epr = set_reg_val(reg->id, val);
+ kvmppc_set_epr(vcpu, new_epr);
break;
}
#if defined(CONFIG_64BIT)
case KVM_REG_PPC_EPCR: {
- u32 new_epcr;
- r = get_user(new_epcr, (u32 __user *)(long)reg->addr);
- if (r == 0)
- kvmppc_set_epcr(vcpu, new_epcr);
+ u32 new_epcr = set_reg_val(reg->id, val);
+ kvmppc_set_epcr(vcpu, new_epcr);
break;
}
#endif
default:
+ r = kvmppc_set_one_reg(vcpu, reg->id, &val);
break;
}
+
return r;
}
diff --git a/arch/powerpc/kvm/e500.c b/arch/powerpc/kvm/e500.c
index 6dd4de7..75875a9 100644
--- a/arch/powerpc/kvm/e500.c
+++ b/arch/powerpc/kvm/e500.c
@@ -425,6 +425,20 @@ int kvmppc_core_set_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs)
return kvmppc_set_sregs_ivor(vcpu, sregs);
}
+int kvmppc_get_one_reg(struct kvm_vcpu *vcpu, u64 id,
+ union kvmppc_one_reg *val)
+{
+ int r = kvmppc_get_one_reg_500_tlb(vcpu, id, val);
+ return r;
+}
+
+int kvmppc_set_one_reg(struct kvm_vcpu *vcpu, u64 id,
+ union kvmppc_one_reg *val)
+{
+ int r = kvmppc_get_one_reg_500_tlb(vcpu, id, val);
+ return r;
+}
+
struct kvm_vcpu *kvmppc_core_vcpu_create(struct kvm *kvm, unsigned int id)
{
struct kvmppc_vcpu_e500 *vcpu_e500;
diff --git a/arch/powerpc/kvm/e500.h b/arch/powerpc/kvm/e500.h
index 41cefd4..fe17f40 100644
--- a/arch/powerpc/kvm/e500.h
+++ b/arch/powerpc/kvm/e500.h
@@ -139,6 +139,10 @@ void kvmppc_e500_tlb_uninit(struct kvmppc_vcpu_e500 *vcpu_e500);
void kvmppc_get_sregs_e500_tlb(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs);
int kvmppc_set_sregs_e500_tlb(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs);
+int kvmppc_get_one_reg_500_tlb(struct kvm_vcpu *vcpu, u64 id,
+ union kvmppc_one_reg *val);
+int kvmppc_set_one_reg_500_tlb(struct kvm_vcpu *vcpu, u64 id,
+ union kvmppc_one_reg *val);
#ifdef CONFIG_KVM_E500V2
unsigned int kvmppc_e500_get_sid(struct kvmppc_vcpu_e500 *vcpu_e500,
diff --git a/arch/powerpc/kvm/e500_mmu.c b/arch/powerpc/kvm/e500_mmu.c
index 66b6e31..b77b855 100644
--- a/arch/powerpc/kvm/e500_mmu.c
+++ b/arch/powerpc/kvm/e500_mmu.c
@@ -596,6 +596,95 @@ int kvmppc_set_sregs_e500_tlb(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs)
return 0;
}
+int kvmppc_get_one_reg_500_tlb(struct kvm_vcpu *vcpu, u64 id,
+ union kvmppc_one_reg *val)
+{
+ int r = 0;
+ long int i;
+
+ switch (id) {
+ case KVM_REG_PPC_MAS0:
+ *val = get_reg_val(id, vcpu->arch.shared->mas0);
+ case KVM_REG_PPC_MAS1:
+ *val = get_reg_val(id, vcpu->arch.shared->mas1);
+ case KVM_REG_PPC_MAS2:
+ *val = get_reg_val(id, vcpu->arch.shared->mas2);
+ case KVM_REG_PPC_MAS7_3:
+ *val = get_reg_val(id, vcpu->arch.shared->mas7_3);
+ case KVM_REG_PPC_MAS4:
+ *val = get_reg_val(id, vcpu->arch.shared->mas4);
+ case KVM_REG_PPC_MAS6:
+ *val = get_reg_val(id, vcpu->arch.shared->mas6);
+ case KVM_REG_PPC_MMUCFG:
+ *val = get_reg_val(id, vcpu->arch.mmucfg);
+ case KVM_REG_PPC_TLB0CFG:
+ case KVM_REG_PPC_TLB1CFG:
+ case KVM_REG_PPC_TLB2CFG:
+ case KVM_REG_PPC_TLB3CFG:
+ i = id - KVM_REG_PPC_TLB0CFG;
+ *val = get_reg_val(id, vcpu->arch.tlbcfg[i]);
+ default:
+ r = -EINVAL;
+ break;
+ }
+
+ return r;
+}
+
+int kvmppc_set_one_reg_500_tlb(struct kvm_vcpu *vcpu, u64 id,
+ union kvmppc_one_reg *val)
+{
+ int r = 0;
+ long int i;
+
+ switch (id) {
+ case KVM_REG_PPC_MAS0:
+ vcpu->arch.shared->mas0 = set_reg_val(id, *val);
+ break;
+ case KVM_REG_PPC_MAS1:
+ vcpu->arch.shared->mas1 = set_reg_val(id, *val);
+ break;
+ case KVM_REG_PPC_MAS2:
+ vcpu->arch.shared->mas2 = set_reg_val(id, *val);
+ break;
+ case KVM_REG_PPC_MAS7_3:
+ vcpu->arch.shared->mas7_3 = set_reg_val(id, *val);
+ break;
+ case KVM_REG_PPC_MAS4:
+ vcpu->arch.shared->mas4 = set_reg_val(id, *val);
+ break;
+ case KVM_REG_PPC_MAS6:
+ vcpu->arch.shared->mas6 = set_reg_val(id, *val);
+ break;
+ case KVM_REG_PPC_MMUCFG: {
+ u32 mmucfg = set_reg_val(id, *val);
+ vcpu->arch.mmucfg = mmucfg & ~MMUCFG_LPIDSIZE;
+ break;
+ }
+ case KVM_REG_PPC_TLB0CFG:
+ case KVM_REG_PPC_TLB1CFG:
+ case KVM_REG_PPC_TLB2CFG:
+ case KVM_REG_PPC_TLB3CFG: {
+ u32 tlbncfg = set_reg_val(id, *val);
+ u32 geometry_mask = TLBnCFG_N_ENTRY | TLBnCFG_ASSOC;
+ i = id - KVM_REG_PPC_TLB0CFG;
+
+ /* MMU geometry (way/size) can be set only using SW_TLB */
+ if ((vcpu->arch.tlbcfg[i] & geometry_mask) !=
+ (tlbncfg & geometry_mask))
+ r = -EINVAL;
+
+ vcpu->arch.tlbcfg[i] = set_reg_val(id, *val);
+ break;
+ }
+ default:
+ r = -EINVAL;
+ break;
+ }
+
+ return r;
+}
+
static int vcpu_mmu_geometry_update(struct kvm_vcpu *vcpu,
struct kvm_book3e_206_tlb_params *params)
{
diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c
index 1f89d26..c255259 100644
--- a/arch/powerpc/kvm/e500mc.c
+++ b/arch/powerpc/kvm/e500mc.c
@@ -255,6 +255,20 @@ int kvmppc_core_set_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs)
return kvmppc_set_sregs_ivor(vcpu, sregs);
}
+int kvmppc_get_one_reg(struct kvm_vcpu *vcpu, u64 id,
+ union kvmppc_one_reg *val)
+{
+ int r = kvmppc_get_one_reg_500_tlb(vcpu, id, val);
+ return r;
+}
+
+int kvmppc_set_one_reg(struct kvm_vcpu *vcpu, u64 id,
+ union kvmppc_one_reg *val)
+{
+ int r = kvmppc_set_one_reg_500_tlb(vcpu, id, val);
+ return r;
+}
+
struct kvm_vcpu *kvmppc_core_vcpu_create(struct kvm *kvm, unsigned int id)
{
struct kvmppc_vcpu_e500 *vcpu_e500;
@@ -337,6 +351,7 @@ static int __init kvmppc_e500mc_init(void)
return kvm_init(NULL, sizeof(struct kvmppc_vcpu_e500), 0, THIS_MODULE);
}
+
static void __exit kvmppc_e500mc_exit(void)
{
kvmppc_booke_exit();
--
1.7.4.1
^ permalink raw reply related
* Re: [PATCH] KVM: PPC: e500: Expose MMU registers via ONE_REG
From: Scott Wood @ 2013-03-19 17:26 UTC (permalink / raw)
To: Mihai Caraman; +Cc: Mihai Caraman, linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <1363713431-27926-1-git-send-email-mihai.caraman@freescale.com>
On 03/19/2013 12:17:11 PM, Mihai Caraman wrote:
> diff --git a/arch/powerpc/kvm/e500_mmu.c b/arch/powerpc/kvm/e500_mmu.c
> index 66b6e31..b77b855 100644
> --- a/arch/powerpc/kvm/e500_mmu.c
> +++ b/arch/powerpc/kvm/e500_mmu.c
> @@ -596,6 +596,95 @@ int kvmppc_set_sregs_e500_tlb(struct kvm_vcpu =20
> *vcpu, struct kvm_sregs *sregs)
> return 0;
> }
>=20
> +int kvmppc_get_one_reg_500_tlb(struct kvm_vcpu *vcpu, u64 id,
> + union kvmppc_one_reg *val)
s/500/e500/
> +int kvmppc_set_one_reg_500_tlb(struct kvm_vcpu *vcpu, u64 id,
> + union kvmppc_one_reg *val)
> +{
> + int r =3D 0;
> + long int i;
> +
> + switch (id) {
> + case KVM_REG_PPC_MAS0:
> + vcpu->arch.shared->mas0 =3D set_reg_val(id, *val);
> + break;
> + case KVM_REG_PPC_MAS1:
> + vcpu->arch.shared->mas1 =3D set_reg_val(id, *val);
> + break;
> + case KVM_REG_PPC_MAS2:
> + vcpu->arch.shared->mas2 =3D set_reg_val(id, *val);
> + break;
> + case KVM_REG_PPC_MAS7_3:
> + vcpu->arch.shared->mas7_3 =3D set_reg_val(id, *val);
> + break;
> + case KVM_REG_PPC_MAS4:
> + vcpu->arch.shared->mas4 =3D set_reg_val(id, *val);
> + break;
> + case KVM_REG_PPC_MAS6:
> + vcpu->arch.shared->mas6 =3D set_reg_val(id, *val);
> + break;
> + case KVM_REG_PPC_MMUCFG: {
> + u32 mmucfg =3D set_reg_val(id, *val);
> + vcpu->arch.mmucfg =3D mmucfg & ~MMUCFG_LPIDSIZE;
> + break;
> + }
Do we really want to allow arbitrary MMUCFG changes? It won't =20
magically make us able to support larger RAs, PIDs, different MAVN, etc.
> + case KVM_REG_PPC_TLB0CFG:
> + case KVM_REG_PPC_TLB1CFG:
> + case KVM_REG_PPC_TLB2CFG:
> + case KVM_REG_PPC_TLB3CFG: {
> + u32 tlbncfg =3D set_reg_val(id, =20
> *val); =09
> + u32 geometry_mask =3D TLBnCFG_N_ENTRY | TLBnCFG_ASSOC;
> + i =3D id - KVM_REG_PPC_TLB0CFG;
> +
> + /* MMU geometry (way/size) can be set only using SW_TLB =20
> */
> + if ((vcpu->arch.tlbcfg[i] & geometry_mask) !=3D
> + (tlbncfg & geometry_mask))
> + r =3D -EINVAL;
> +
> + vcpu->arch.tlbcfg[i] =3D set_reg_val(id, *val);
> + break;
> + }
Likewise -- just because QEMU sets a bit here doesn't mean KVM can =20
support it.
I thought the initial plan for setting these config registers was to =20
accept it if it exactly matches what KVM already has, and give an error =20
otherwise -- thus allowing for the possibliity of accepting certain =20
specific updates in the future.
-Scott=
^ permalink raw reply
* Re: [PATCH2/11] Add PRRN Event Handler
From: Nathan Fontenot @ 2013-03-19 18:01 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev
In-Reply-To: <20130314085155.GB9841@iris.ozlabs.ibm.com>
On 03/14/2013 03:51 AM, Paul Mackerras wrote:
> On Fri, Mar 08, 2013 at 10:00:09PM -0600, Nathan Fontenot wrote:
>> From: Jesse Larrew <jlarrew@linux.vnet.ibm.com>
>>
>> A PRRN event is signaled via the RTAS event-scan mechanism, which
>> returns a Hot Plug Event message "fixed part" indicating "Platform
>> Resource Reassignment". In response to the Hot Plug Event message,
>> we must call ibm,update-nodes to determine which resources were
>> reassigned and then ibm,update-properties to obtain the new affinity
>> information about those resources.
>>
>> The PRRN event-scan RTAS message contains only the "fixed part" with
>> the "Type" field set to the value 160 and no Extended Event Log. The
>> four-byte Extended Event Log Length field is repurposed (since no
>> Extended Event Log message is included) to pass the "scope" parameter
>> that causes the ibm,update-nodes to return the nodes affected by the
>> specific resource reassignment.
>>
>> This patch adds a handler in rtasd for PRRN RTAS events. The function
>> pseries_devicetree_update() (from mobility.c) is used to make the
>> ibm,update-nodes/ibm,update-properties RTAS calls. Updating the NUMA maps
>> (handled by a subsequent patch) will require significant processing,
>> so pseries_devicetree_update() is called from an asynchronous workqueue
>> to allow rtasd to continue processing events.
>>
>> Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
>
> [snip]
>
>> +static s32 update_scope;
>
> Do we have a guarantee that there can only be one of these events
> outstanding at a time? If so it would be nice to document that in a
> comment next to this declaration, so we know in future that this is
> why this is safe.
>
We only allow for one event to be outstanding. When a PRRN Event is
received we flush any work currently queued up and add the new event
event to the workqueue (see prrn_schedule_work() from the patch).
As I understand flush_work(), this would wait for any work in flight
to complete, then remove all work before returning. I'll add a comment
and update the patch description.
-Nathan
^ permalink raw reply
* Re: [PATCH 4/11] Add platform_has_feature()
From: Nathan Fontenot @ 2013-03-19 18:03 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev
In-Reply-To: <20130314085616.GD9841@iris.ozlabs.ibm.com>
On 03/14/2013 03:56 AM, Paul Mackerras wrote:
> On Fri, Mar 08, 2013 at 10:02:31PM -0600, Nathan Fontenot wrote:
>> The firmware_has_feature() function makes it easy to check for supported
>> features of the hardware. There is not corresponding function to check for
>> features supported by the client architecture.
>
> Actually, firmware_has_feature checks for supported features of the
> hypervisor, or in a sense the platform, rather than hardware.
Ahh, thanks for clarifying that for me. I'll update the description.
>
>> This patch adds a platform_has_feature() function to check features selected
>> by firmware and reported via the device tree 'ibm,architecture-vec5'
>> property. As part of this the #defines used for the architecture vector are
>> moved to prom.h and re-defined such that the vector 5 options have the vector
>> index and the feature bits encoded into them. This allows for callers of
>> platform_has_feature() to pass in a single pre-defined value.
>
> One other comment below...
>
>> /* PCIe/MSI support. Without MSI full PCIe is not supported */
>> #ifdef CONFIG_PCI_MSI
>> -#define OV5_MSI 0x01 /* PCIe/MSI support */
>> +#define OV5_MSI 0x0201 /* PCIe/MSI support */
>> #else
>> -#define OV5_MSI 0x00
>> +#define OV5_MSI 0x0200
>> #endif /* CONFIG_PCI_MSI */
>
> The #ifdef was done this way in order to control what ended up in the
> option vector we pass to the platform firmware. For checking what the
> platform supports, wouldn't we want OV5_MSI to be 0x0201 always?
> Similarly for OV5_CMO, OV5_XCMO, etc.?
Yes, you're correct. I will update this.
--
-Nathan
^ permalink raw reply
* Re: [PATCH 4/11] Add platform_has_feature()
From: Nathan Fontenot @ 2013-03-19 18:05 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev
In-Reply-To: <20130314085905.GE9841@iris.ozlabs.ibm.com>
On 03/14/2013 03:59 AM, Paul Mackerras wrote:
> On Fri, Mar 08, 2013 at 10:02:31PM -0600, Nathan Fontenot wrote:
>> This patch adds a platform_has_feature() function to check features selected
>> by firmware and reported via the device tree 'ibm,architecture-vec5'
>> property. As part of this the #defines used for the architecture vector are
>> moved to prom.h and re-defined such that the vector 5 options have the vector
>> index and the feature bits encoded into them. This allows for callers of
>> platform_has_feature() to pass in a single pre-defined value.
>
> One other comment...
>
>> +#if defined(CONFIG_PPC_PSERIES) || defined(CONFIG_PPC_POWERNV)
>> +bool platform_has_feature(unsigned int feature)
>> +{
>> + struct device_node *chosen;
>> + const char *vec5;
>> + bool has_option;
>> +
>> + chosen = of_find_node_by_path("/chosen");
>> + if (!chosen)
>> + return false;
>> +
>> + vec5 = of_get_property(chosen, "ibm,architecture-vec-5", NULL);
>> + has_option = vec5 && (vec5[OV5_INDX(feature)] & OV5_FEAT(feature));
>
> You access vec5[index] without checking that the vector is at least
> index+1 bytes long, according to either the length byte at the
> beginning of the vector, or the total length of the property.
> Checking both would be a good idea.
>
Yep. Thanks for letting me know.
--
-Nathan
^ permalink raw reply
* Re: [PATCH 4/11] Add platform_has_feature()
From: Nathan Fontenot @ 2013-03-19 18:15 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <20130314134212.GA6736@concordia>
On 03/14/2013 08:42 AM, Michael Ellerman wrote:
> On Fri, Mar 08, 2013 at 10:02:31PM -0600, Nathan Fontenot wrote:
>> The firmware_has_feature() function makes it easy to check for supported
>> features of the hardware. There is not corresponding function to check for
>> features supported by the client architecture.
>
> Actually it doesn't tell you about features of the hardware, it tells
> you about features of the firmware, or the platform ..
>
> So I think you should really just be adding a new firmware feature flag,
> and adding whatever glue code is required to set it based on what you
> find in the device tree.
>
> Also notice where you end up using it:
>
> - if (firmware_has_feature(FW_FEATURE_OPAL))
> + if (firmware_has_feature(FW_FEATURE_OPAL) ||
> + platform_has_feature(OV5_TYPE1_AFFINITY)) {
> + dbg("Using form 1 affinity\n");
> form1_affinity = 1;
>
> Could be:
>
> + if (firmware_has_feature(FW_FEATURE_FORM1_AFFINITY) ||
>
To make sure I understand what you're suggesting...
You think there should be a single firmware_has_feature() for all current
uses and also for checking items such as FORM1_AFFINITY and PRRN
features as reported by the device tree for vector 5 portions of the
client architecture bits. I think this could be done by checking the
device tree ibm,architecture-vec-5 node for a specified feature and
setting a bit the appropriate bit in powerpc_firmware_features.
I like this more than separate firmware_has_feature() and platform_has_feature()
routines to check.
--
-Nathan
^ permalink raw reply
* [PATCH] [RFC] powerpc: Add VDSO version of time
From: Adhemerval Zanella @ 2013-03-19 19:55 UTC (permalink / raw)
To: linuxppc-dev
Hi all,
This patch implement the time syscall as vDSO. I have a glibc patch
to use it as IFUNC (as latest gettimeofday patch). Below the perf
numbers:
Baseline PPC32: 380 nsec
Baseline PPC64: 352 nsec
vdso PPC32: 20 nsec
vdso PPC64: 20 nsec
I focused on 64 bit kernel, do I need to provide a scheme for 32 bits
as well?
Any tips, advices, comments?
--
diff --git a/arch/powerpc/kernel/vdso.c b/arch/powerpc/kernel/vdso.c
index 1b2076f..d4f463a 100644
--- a/arch/powerpc/kernel/vdso.c
+++ b/arch/powerpc/kernel/vdso.c
@@ -113,6 +113,10 @@ static struct vdso_patch_def vdso_patches[] = {
CPU_FTR_USE_TB, 0,
"__kernel_get_tbfreq", NULL
},
+ {
+ CPU_FTR_USE_TB, 0,
+ "__kernel_time", NULL
+ },
};
/*
diff --git a/arch/powerpc/kernel/vdso32/gettimeofday.S b/arch/powerpc/kernel/vdso32/gettimeofday.S
index 4ee09ee..9a60a87 100644
--- a/arch/powerpc/kernel/vdso32/gettimeofday.S
+++ b/arch/powerpc/kernel/vdso32/gettimeofday.S
@@ -181,6 +181,35 @@ V_FUNCTION_END(__kernel_clock_getres)
/*
+ * Exact prototype of time()
+ *
+ * time_t time(time *t);
+ *
+ */
+V_FUNCTION_BEGIN(__kernel_time)
+ .cfi_startproc
+ mflr r12
+ .cfi_register lr,r12
+
+ mr r11,r3 /* r11 holds t */
+ bl __get_datapage@local
+ mr r9, r3 /* datapage ptr in r9 */
+
+1: lwz r8,(CFG_TB_UPDATE_COUNT+LOPART)(r9)
+ lwz r3,STAMP_XTIME+TSPEC_TV_SEC(r9)
+ andi. r0,r8,1 /* pending update ? loop */
+ bne- 1b
+
+ cmplwi r11,0 /* check if t is NULL */
+ beq 2f
+ stw r3,0(r11) /* store result at *t */
+2: mtlr r12
+ crclr cr0*4+so
+ blr
+ .cfi_endproc
+V_FUNCTION_END(__kernel_time)
+
+/*
* This is the core of clock_gettime() and gettimeofday(),
* it returns the current time in r3 (seconds) and r4.
* On entry, r7 gives the resolution of r4, either USEC_PER_SEC
diff --git a/arch/powerpc/kernel/vdso32/vdso32.lds.S b/arch/powerpc/kernel/vdso32/vdso32.lds.S
index 43200ba..f223409 100644
--- a/arch/powerpc/kernel/vdso32/vdso32.lds.S
+++ b/arch/powerpc/kernel/vdso32/vdso32.lds.S
@@ -150,6 +150,7 @@ VERSION
#ifdef CONFIG_PPC64
__kernel_getcpu;
#endif
+ __kernel_time;
local: *;
};
diff --git a/arch/powerpc/kernel/vdso64/gettimeofday.S b/arch/powerpc/kernel/vdso64/gettimeofday.S
index e97a9a0..f05aa68 100644
--- a/arch/powerpc/kernel/vdso64/gettimeofday.S
+++ b/arch/powerpc/kernel/vdso64/gettimeofday.S
@@ -164,6 +164,35 @@ V_FUNCTION_BEGIN(__kernel_clock_getres)
.cfi_endproc
V_FUNCTION_END(__kernel_clock_getres)
+/*
+ * Exact prototype of time()
+ *
+ * time_t time(time *t);
+ *
+ */
+V_FUNCTION_BEGIN(__kernel_time)
+ .cfi_startproc
+ mflr r12
+ .cfi_register lr,r12
+
+ mr r11,r3 /* r11 holds t */
+ bl V_LOCAL_FUNC(__get_datapage)
+
+1: ld r8,CFG_TB_UPDATE_COUNT(r3)
+ ld r4,STAMP_XTIME+TSPC64_TV_SEC(r3)
+ andi. r0,r8,1 /* pending update ? loop */
+ bne- 1b
+
+ cmpldi r11,0 /* check if t is NULL */
+ beq 2f
+ std r4,0(r11) /* store result at *t */
+2: mtlr r12
+ crclr cr0*4+so
+ mr r3,r4
+ blr
+ .cfi_endproc
+V_FUNCTION_END(__kernel_time)
+
/*
* This is the core of clock_gettime() and gettimeofday(),
diff --git a/arch/powerpc/kernel/vdso64/vdso64.lds.S b/arch/powerpc/kernel/vdso64/vdso64.lds.S
index e6c1758..e486381 100644
--- a/arch/powerpc/kernel/vdso64/vdso64.lds.S
+++ b/arch/powerpc/kernel/vdso64/vdso64.lds.S
@@ -147,6 +147,7 @@ VERSION
__kernel_sync_dicache_p5;
__kernel_sigtramp_rt64;
__kernel_getcpu;
+ __kernel_time;
local: *;
};
^ permalink raw reply related
* Re: [PATCH] powerpc: add Book E support to 64-bit hibernation
From: Johannes Berg @ 2013-03-19 20:55 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev, Wang Dongsheng
In-Reply-To: <1363644757.27435.16@snotra>
On Mon, 2013-03-18 at 17:12 -0500, Scott Wood wrote:
> Could you elaborate on why book3s flushes the way it does? What's
> special about the first 32 MiB? If it's to cover kernel code, why
> would that be changing from what's already there?
I was going to say I have no idea, but looking at it again ... this is
in the *resume* code, not the suspend code as I'd assumed, and on resume
I guess I felt it was safer to not assume it didn't change, since it
could be a slightly different kernel that loaded and restored the
hibernation image? It should be the same one, so I guess it should be
exactly the same code, but I guess I wanted to make sure there wasn't
anything weird there. As for why it'd be 32 MiB? No idea. Although that
really ought to flush all your possible caches anyway, I guess.
johannes
^ permalink raw reply
* Re: Build regressions/improvements in v3.9-rc3
From: Geert Uytterhoeven @ 2013-03-19 20:52 UTC (permalink / raw)
To: Linux Kernel Development; +Cc: linux-s390, Linux/PPC Development
In-Reply-To: <alpine.DEB.2.00.1303192148530.18971@ayla.of.borg>
On Tue, 19 Mar 2013, Geert Uytterhoeven wrote:
> JFYI, when comparing v3.9-rc3 to v3.9-rc2[3], the summaries are:
> - build errors: +10/-12
10 regressions:
+ arch/powerpc/kvm/book3s_hv.c: error: implicit declaration of function 'inhibit_secondary_onlining' [-Werror=implicit-function-declaration]: => 1855:2
+ arch/powerpc/kvm/book3s_hv.c: error: implicit declaration of function 'uninhibit_secondary_onlining' [-Werror=implicit-function-declaration]: => 1862:2
ppc64_defconfig+UP
+ error: "devm_request_threaded_irq" [drivers/dma/dw_dmac.ko] undefined!: => N/A
+ error: "devm_request_threaded_irq" [drivers/i2c/busses/i2c-ocores.ko] undefined!: => N/A
+ error: "devm_request_threaded_irq" [drivers/i2c/i2c-smbus.ko] undefined!: => N/A
+ error: "devm_request_threaded_irq" [drivers/media/platform/sh_veu.ko] undefined!: => N/A
+ error: "devm_request_threaded_irq" [drivers/spi/spi-altera.ko] undefined!: => N/A
+ error: "mfd_add_devices" [drivers/mfd/lpc_sch.ko] undefined!: => N/A
+ error: "mfd_remove_devices" [drivers/mfd/lpc_sch.ko] undefined!: => N/A
s390-allmodconfig
+ error: "vgacon_remap_base" [drivers/video/vga16fb.ko] undefined!: => N/A
powerpc-randconfig
> [1] http://kisskb.ellerman.id.au/kisskb/head/5993/ (118 out of 117 configs)
> [3] http://kisskb.ellerman.id.au/kisskb/head/5972/ (all 117 configs)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH] powerpc: add Book E support to 64-bit hibernation
From: Scott Wood @ 2013-03-19 21:10 UTC (permalink / raw)
To: Johannes Berg; +Cc: linuxppc-dev, Wang Dongsheng
In-Reply-To: <1363726534.8336.37.camel@jlt4.sipsolutions.net>
On 03/19/2013 03:55:34 PM, Johannes Berg wrote:
> On Mon, 2013-03-18 at 17:12 -0500, Scott Wood wrote:
>=20
> > Could you elaborate on why book3s flushes the way it does? What's
> > special about the first 32 MiB? If it's to cover kernel code, why
> > would that be changing from what's already there?
>=20
> I was going to say I have no idea, but looking at it again ... this is
> in the *resume* code, not the suspend code as I'd assumed, and on =20
> resume
> I guess I felt it was safer to not assume it didn't change, since it
> could be a slightly different kernel that loaded and restored the
> hibernation image?
Wouldn't that be doomed for other reasons?
I wonder about kernel modules, though flushing 32 MiB wouldn't be =20
adequate there.
> It should be the same one, so I guess it should be
> exactly the same code, but I guess I wanted to make sure there wasn't
> anything weird there. As for why it'd be 32 MiB? No idea. Although =20
> that
> really ought to flush all your possible caches anyway, I guess.
It's not a displacement flush (i.e. you don't do a separate load pass =20
first) -- it just flushes lines if they happen to be present, and =20
leaves alone anything outside that range. Given that you just finished =20
copying a bunch of data, most likely what's in the cache is the last =20
bit of data you copied.
-Scott=
^ permalink raw reply
* Re: [PATCH] powerpc: add Book E support to 64-bit hibernation
From: Johannes Berg @ 2013-03-19 21:22 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev, Wang Dongsheng
In-Reply-To: <1363727445.16671.30@snotra>
On Tue, 2013-03-19 at 16:10 -0500, Scott Wood wrote:
> > I was going to say I have no idea, but looking at it again ... this is
> > in the *resume* code, not the suspend code as I'd assumed, and on
> > resume
> > I guess I felt it was safer to not assume it didn't change, since it
> > could be a slightly different kernel that loaded and restored the
> > hibernation image?
>
> Wouldn't that be doomed for other reasons?
Most likely, yeah.
> I wonder about kernel modules, though flushing 32 MiB wouldn't be
> adequate there.
Good question, but would they be running? You have to have everything
built in that you need to load the image? Or maybe not, with the
userspace image restoration that became possible at some point...
> > It should be the same one, so I guess it should be
> > exactly the same code, but I guess I wanted to make sure there wasn't
> > anything weird there. As for why it'd be 32 MiB? No idea. Although
> > that
> > really ought to flush all your possible caches anyway, I guess.
>
> It's not a displacement flush (i.e. you don't do a separate load pass
> first) -- it just flushes lines if they happen to be present, and
> leaves alone anything outside that range. Given that you just finished
> copying a bunch of data, most likely what's in the cache is the last
> bit of data you copied.
Oops, good point.
Maybe there's a way to completely flush the (i)cache? :-)
johannes
^ permalink raw reply
* Re: [PATCH] powerpc: add Book E support to 64-bit hibernation
From: Scott Wood @ 2013-03-19 22:16 UTC (permalink / raw)
To: Johannes Berg; +Cc: linuxppc-dev, Wang Dongsheng
In-Reply-To: <1363728127.8336.39.camel@jlt4.sipsolutions.net>
On 03/19/2013 04:22:07 PM, Johannes Berg wrote:
> On Tue, 2013-03-19 at 16:10 -0500, Scott Wood wrote:
>=20
> > > I was going to say I have no idea, but looking at it again ... =20
> this is
> > > in the *resume* code, not the suspend code as I'd assumed, and on
> > > resume
> > > I guess I felt it was safer to not assume it didn't change, since =20
> it
> > > could be a slightly different kernel that loaded and restored the
> > > hibernation image?
> >
> > Wouldn't that be doomed for other reasons?
>=20
> Most likely, yeah.
>=20
> > I wonder about kernel modules, though flushing 32 MiB wouldn't be
> > adequate there.
>=20
> Good question, but would they be running? You have to have everything
> built in that you need to load the image? Or maybe not, with the
> userspace image restoration that became possible at some point...
Is that all that's being restored in this step, or would we be loading =20
all modules that were loaded before suspend (as they're normally not =20
swappable)? I'm not too familiar with what gets saved where.
> > It's not a displacement flush (i.e. you don't do a separate load =20
> pass
> > first) -- it just flushes lines if they happen to be present, and
> > leaves alone anything outside that range. Given that you just =20
> finished
> > copying a bunch of data, most likely what's in the cache is the last
> > bit of data you copied.
>=20
> Oops, good point.
>=20
> Maybe there's a way to completely flush the (i)cache? :-)
There is, but it's platform-dependent, and not pleasant on our chips =20
(need a displacement flush for L1, and sometimes errata are involved =20
for L2).
-Scott=
^ permalink raw reply
* Re: [PATCH 3/3] powerpc/fsl: add MPIC timer wakeup support
From: Scott Wood @ 2013-03-19 22:54 UTC (permalink / raw)
To: Wang Dongsheng-B40534
Cc: Wood Scott-B07421, Gala Kumar-B11780,
linuxppc-dev@lists.ozlabs.org, Li Yang-R58472,
Zhao Chenhui-B35336
In-Reply-To: <ABB05CD9C9F68C46A5CEDC7F15439259EB7111@039-SN2MPN1-022.039d.mgd.msft.net>
On 03/19/2013 01:25:42 AM, Wang Dongsheng-B40534 wrote:
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Tuesday, March 19, 2013 8:31 AM
> > To: Wang Dongsheng-B40534
> > Cc: Gala Kumar-B11780; linuxppc-dev@lists.ozlabs.org; Wang =20
> Dongsheng-
> > B40534; Zhao Chenhui-B35336; Li Yang-R58472
> > Subject: Re: [PATCH 3/3] powerpc/fsl: add MPIC timer wakeup support
> >
> > On 03/08/2013 01:38:47 AM, Wang Dongsheng wrote:
> > > +static ssize_t fsl_timer_wakeup_store(struct device *dev,
> > > + struct device_attribute *attr,
> > > + const char *buf,
> > > + size_t count)
> > > +{
> > > + struct timeval interval;
> > > + int ret;
> > > +
> > > + interval.tv_usec =3D 0;
> > > + if (kstrtol(buf, 0, &interval.tv_sec))
> > > + return -EINVAL;
> >
> > I don't think the buffer will NUL-terminated... Ordinarily =20
> there'll be
> > an LF terminator, but you can't rely on that (many other sysfs =20
> attributes
> > seem to, though...).
> >
> I think we don't need to care about LF terminator.
> The kstrtol--> _kstrtoull has been done.
My point is, what happens if userspace passes in a buffer that has no =20
terminator of any sort? kstrtol will continue reading beyond the end =20
of the buffer.
> > > + mutex_lock(&sysfs_lock);
> > > +
> > > + if (fsl_wakeup->timer && !interval.tv_sec) {
> > > + disable_irq_wake(fsl_wakeup->timer->irq);
> > > + mpic_free_timer(fsl_wakeup->timer);
> > > + fsl_wakeup->timer =3D NULL;
> > > + mutex_unlock(&sysfs_lock);
> > > +
> > > + return count;
> > > + }
> > > +
> > > + if (fsl_wakeup->timer) {
> > > + mutex_unlock(&sysfs_lock);
> > > + return -EBUSY;
> > > + }
> >
> > So to change an already-set timer you have to set it to zero and =20
> then to
> > what you want? Why not just do:
> >
> > if (fsl_wakeup->timer) {
> > disable_irq_wake(...);
> > mpic_free_timer(...);
> > fsl_wakeup_timer =3D NULL;
> > }
> >
> > if (!interval.tv_sec) {
> > mutex_unlock(&sysfs_lock);
> > return count;
> > }
> >
> You can't break up the it.
> if echo zero the code will cancel the timer that is currently running.
> Not echo non-zero value just zero to cancel.
Echoing a nonzero value wouldn't just be to cancel, it would be to set =20
a new timer after cancelling the old.
> > > + for (i =3D 0; mpic_attributes[i]; i++) {
> > > + ret =3D device_create_file(mpic_subsys.dev_root,
> > > + mpic_attributes[i]);
> > > + if (ret)
> > > + goto err2;
> > > + }
> >
> > Is this code ever going to register more than one?
> >
> No, just one. I only keep the style here.
> If you don't think it's necessary I can remove this loop.
I don't think it's necessary.
-Scott=
^ permalink raw reply
* Re: [PATCH 2/3] powerpc/mpic: add global timer support
From: Scott Wood @ 2013-03-19 22:59 UTC (permalink / raw)
To: Wang Dongsheng-B40534
Cc: Wood Scott-B07421, Gala Kumar-B11780,
linuxppc-dev@lists.ozlabs.org, Li Yang-R58472
In-Reply-To: <ABB05CD9C9F68C46A5CEDC7F15439259EB717E@039-SN2MPN1-022.039d.mgd.msft.net>
On 03/19/2013 02:55:58 AM, Wang Dongsheng-B40534 wrote:
> > > +static void convert_ticks_to_time(struct timer_group_priv *priv,
> > > + const u64 ticks, struct timeval *time) {
> > > + u64 tmp_sec;
> > > + u32 rem_us;
> > > + u32 div;
> > > +
> > > + if (!(priv->flags & FSL_GLOBAL_TIMER)) {
> > > + time->tv_sec =3D (__kernel_time_t)
> > > + div_u64_rem(ticks, priv->timerfreq, &rem_us);
> > > + tmp_sec =3D (u64)time->tv_sec * (u64)priv->timerfreq;
> > > + time->tv_usec =3D (__kernel_suseconds_t)
> > > + div_u64((ticks - tmp_sec) * 1000000,
> > > priv->timerfreq);
> > > +
> > > + return;
> > > + }
> > > +
> > > + div =3D (1 << (MPIC_TIMER_TCR_CLKDIV_64 >> 8)) * 8;
> > > +
> > > + time->tv_sec =3D (__kernel_time_t)div_u64(ticks, priv->timerfreq
> > > / div);
> > > + tmp_sec =3D div_u64((u64)time->tv_sec * (u64)priv->timerfreq,
> > > div);
> > > +
> > > + time->tv_usec =3D (__kernel_suseconds_t)
> > > + div_u64((ticks - tmp_sec) * 1000000, priv->timerfreq /
> > > div);
> > > +
> > > + return;
> >
> > Why don't you just adjust the clock frequency up front for =20
> CLKDIV_64,
> > rather than introduce alternate (and untested!) code paths =20
> throughout the
> > driver?
> >
> No, It cannot be integrated. The div cannot be removed.
> Because if do priv->timerfreq /=3D div, that will affect the accuracy.
>=20
> Like:
> 3 * 5 / 2 =3D 7;
> 3 / 2 * 5 =3D 5;
I don't follow -- a change in the clock speed is a change in the clock =20
speed, no matter how you accomplish it.
How you round is a different question. You should probably be rounding =20
up always, based on the final clock frequency -- though it's unlikely =20
to matter much given the high precision of the timer relative to the =20
input granularity.
> BTW
> if (!(priv->flags & FSL_GLOBAL_TIMER)) {
> time->tv_sec =3D (__kernel_time_t)
> div_u64_rem(ticks, priv->timerfreq, &rem_us);
> tmp_sec =3D (u64)time->tv_sec * (u64)priv->timerfreq;
> time->tv_usec =3D (__kernel_suseconds_t)
> div_u64((ticks - tmp_sec) * 1000000, priv->timerfreq);
>=20
> return;
> }
> This branch I has been tested.
>=20
> Test methods:
> 1. Get timerfreq and set timerfreq.
> timerfreq /=3D 64;(Clock ratio is divide by 64)
>=20
> 2. Clear FSL_GLOBAL_TIMER flag.
Even if it was tested once, it's unlikely to continue to be tested =20
without a user.
-Scott=
^ permalink raw reply
* RE: [PATCH 3/3] powerpc/fsl: add MPIC timer wakeup support
From: Wang Dongsheng-B40534 @ 2013-03-20 3:48 UTC (permalink / raw)
To: Wood Scott-B07421
Cc: Gala Kumar-B11780, linuxppc-dev@lists.ozlabs.org, Li Yang-R58472,
Zhao Chenhui-B35336
In-Reply-To: <1363733672.16671.34@snotra>
> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Wednesday, March 20, 2013 6:55 AM
> To: Wang Dongsheng-B40534
> Cc: Wood Scott-B07421; Gala Kumar-B11780; linuxppc-dev@lists.ozlabs.org;
> Zhao Chenhui-B35336; Li Yang-R58472
> Subject: Re: [PATCH 3/3] powerpc/fsl: add MPIC timer wakeup support
>=20
> On 03/19/2013 01:25:42 AM, Wang Dongsheng-B40534 wrote:
> > > -----Original Message-----
> > > From: Wood Scott-B07421
> > > Sent: Tuesday, March 19, 2013 8:31 AM
> > > To: Wang Dongsheng-B40534
> > > Cc: Gala Kumar-B11780; linuxppc-dev@lists.ozlabs.org; Wang
> > Dongsheng-
> > > B40534; Zhao Chenhui-B35336; Li Yang-R58472
> > > Subject: Re: [PATCH 3/3] powerpc/fsl: add MPIC timer wakeup support
> > >
> > > On 03/08/2013 01:38:47 AM, Wang Dongsheng wrote:
> > > > +static ssize_t fsl_timer_wakeup_store(struct device *dev,
> > > > + struct device_attribute *attr,
> > > > + const char *buf,
> > > > + size_t count)
> > > > +{
> > > > + struct timeval interval;
> > > > + int ret;
> > > > +
> > > > + interval.tv_usec =3D 0;
> > > > + if (kstrtol(buf, 0, &interval.tv_sec))
> > > > + return -EINVAL;
> > >
> > > I don't think the buffer will NUL-terminated... Ordinarily
> > there'll be
> > > an LF terminator, but you can't rely on that (many other sysfs
> > attributes
> > > seem to, though...).
> > >
> > I think we don't need to care about LF terminator.
> > The kstrtol--> _kstrtoull has been done.
>=20
> My point is, what happens if userspace passes in a buffer that has no
> terminator of any sort? kstrtol will continue reading beyond the end of
> the buffer.
>=20
Do not care about terminator.
kstrtol--> _kstrtoull--> _parse_integer
_kstrtoull(...) {
...
rv =3D _parse_integer(s, base, &_res);
if (rv & KSTRTOX_OVERFLOW)
return -ERANGE;
rv &=3D ~KSTRTOX_OVERFLOW;
if (rv =3D=3D 0)
return -EINVAL;
s +=3D rv;
if (*s =3D=3D '\n')
s++;
if (*s)
return -EINVAL;
...
}
_parse_integer(...) {
...
while (*s) {
if ('0' <=3D *s && *s <=3D '9')
val =3D *s - '0';
else if ('a' <=3D _tolower(*s) && _tolower(*s) <=3D 'f')
val =3D _tolower(*s) - 'a' + 10;
else
break; //this will break out to convert.
if (val >=3D base)
break;
}
...
}
> > > > + mutex_lock(&sysfs_lock);
> > > > +
> > > > + if (fsl_wakeup->timer && !interval.tv_sec) {
> > > > + disable_irq_wake(fsl_wakeup->timer->irq);
> > > > + mpic_free_timer(fsl_wakeup->timer);
> > > > + fsl_wakeup->timer =3D NULL;
> > > > + mutex_unlock(&sysfs_lock);
> > > > +
> > > > + return count;
> > > > + }
> > > > +
> > > > + if (fsl_wakeup->timer) {
> > > > + mutex_unlock(&sysfs_lock);
> > > > + return -EBUSY;
> > > > + }
> > >
> > > So to change an already-set timer you have to set it to zero and
> > then to
> > > what you want? Why not just do:
> > >
> > > if (fsl_wakeup->timer) {
> > > disable_irq_wake(...);
> > > mpic_free_timer(...);
> > > fsl_wakeup_timer =3D NULL;
> > > }
> > >
> > > if (!interval.tv_sec) {
> > > mutex_unlock(&sysfs_lock);
> > > return count;
> > > }
> > >
> > You can't break up the it.
> > if echo zero the code will cancel the timer that is currently running.
> > Not echo non-zero value just zero to cancel.
>=20
> Echoing a nonzero value wouldn't just be to cancel, it would be to set a
> new timer after cancelling the old.
>=20
If you think this way is better, I can change.
But why should do it?=20
Explicitly stop the timer (echo 0) before reuse it is more reasonable for m=
e.=20
> > > > + for (i =3D 0; mpic_attributes[i]; i++) {
> > > > + ret =3D device_create_file(mpic_subsys.dev_root,
> > > > + mpic_attributes[i]);
> > > > + if (ret)
> > > > + goto err2;
> > > > + }
> > >
> > > Is this code ever going to register more than one?
> > >
> > No, just one. I only keep the style here.
> > If you don't think it's necessary I can remove this loop.
>=20
> I don't think it's necessary.
>=20
Ok, I will remove this loop.
^ permalink raw reply
* Re: [PATCH] [RFC] powerpc: Add VDSO version of time
From: Benjamin Herrenschmidt @ 2013-03-20 5:00 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: linuxppc-dev
In-Reply-To: <5148C2B3.6010408@linux.vnet.ibm.com>
On Tue, 2013-03-19 at 16:55 -0300, Adhemerval Zanella wrote:
>
> I focused on 64 bit kernel, do I need to provide a scheme for 32 bits
> as well?
You did provide both 32 and 64-bit VDSO implementations so 32-bit
kernels should be covered.
Cheers,
Ben.
^ permalink raw reply
* [PATCHv2 3/3] ppc64: implemented pcibios_get_speed_cap_mask
From: Lucas Kannebley Tavares @ 2013-03-20 5:24 UTC (permalink / raw)
To: linux-pci, linuxppc-dev, dri-devel
Cc: David Airlie, Brian King, Thadeu Cascardo,
Lucas Kannebley Tavares, Bjorn Helgaas, Alex Deucher
In-Reply-To: <1363757079-23550-1-git-send-email-lucaskt@linux.vnet.ibm.com>
Implementation of a architecture-specific pcibios_get_speed_cap_mask.
This implementation detects bus capabilities based on OF
ibm,pcie-link-speed-stats property.
Signed-off-by: Lucas Kannebley Tavares <lucaskt@linux.vnet.ibm.com>
---
arch/powerpc/platforms/pseries/pci.c | 35 ++++++++++++++++++++++++++++++++++
1 files changed, 35 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/pci.c b/arch/powerpc/platforms/pseries/pci.c
index 0b580f4..4da8deb 100644
--- a/arch/powerpc/platforms/pseries/pci.c
+++ b/arch/powerpc/platforms/pseries/pci.c
@@ -24,6 +24,7 @@
#include <linux/kernel.h>
#include <linux/pci.h>
#include <linux/string.h>
+#include <linux/device.h>
#include <asm/eeh.h>
#include <asm/pci-bridge.h>
@@ -108,3 +109,37 @@ static void fixup_winbond_82c105(struct pci_dev* dev)
}
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_WINBOND, PCI_DEVICE_ID_WINBOND_82C105,
fixup_winbond_82c105);
+
+int pcibios_get_speed_cap_mask(struct pci_dev *dev, u32 *mask)
+{
+ struct device_node *dn, *pdn;
+ const uint32_t *pcie_link_speed_stats = NULL;
+
+ *mask = 0;
+ dn = pci_bus_to_OF_node(dev->bus);
+
+ /* Find nearest ibm,pcie-link-speed-stats, walking up the device tree */
+ for (pdn = dn; pdn != NULL; pdn = pdn->parent) {
+ pcie_link_speed_stats = (const uint32_t *) of_get_property(pdn,
+ "ibm,pcie-link-speed-stats", NULL);
+ if (pcie_link_speed_stats != NULL)
+ break;
+ }
+
+ if (pcie_link_speed_stats == NULL) {
+ dev_info(&dev->dev, "no ibm,pcie-link-speed-stats property\n");
+ return -EINVAL;
+ }
+
+ switch (pcie_link_speed_stats[0]) {
+ case 0x02:
+ *mask |= PCIE_SPEED_50;
+ case 0x01:
+ *mask |= PCIE_SPEED_25;
+ case 0x00:
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
--
1.7.4.4
^ permalink raw reply related
* [PATCHv2 2/3] drm: removed drm_pcie_get_speed_cap_mask function
From: Lucas Kannebley Tavares @ 2013-03-20 5:24 UTC (permalink / raw)
To: linux-pci, linuxppc-dev, dri-devel
Cc: David Airlie, Brian King, Thadeu Cascardo,
Lucas Kannebley Tavares, Bjorn Helgaas, Alex Deucher
In-Reply-To: <1363757079-23550-1-git-send-email-lucaskt@linux.vnet.ibm.com>
This function was moved to the pci subsystem where it fits better, as
this is much more of a generic pci task, than a drm specific one. All
references to the function (all in the radeon driver) are updated.
This is the second step in moving function drm_pcie_get_speed_cap_mask
from the drm subsystem to the pci one.
Signed-off-by: Lucas Kannebley Tavares <lucaskt@linux.vnet.ibm.com>
---
drivers/gpu/drm/drm_pci.c | 38 ------------------------------------
drivers/gpu/drm/radeon/evergreen.c | 5 ++-
drivers/gpu/drm/radeon/r600.c | 5 ++-
drivers/gpu/drm/radeon/rv770.c | 5 ++-
include/drm/drmP.h | 6 -----
5 files changed, 9 insertions(+), 50 deletions(-)
diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index bd719e9..ba70844 100644
--- a/drivers/gpu/drm/drm_pci.c
+++ b/drivers/gpu/drm/drm_pci.c
@@ -439,44 +439,6 @@ int drm_pci_init(struct drm_driver *driver, struct pci_driver *pdriver)
return 0;
}
-int drm_pcie_get_speed_cap_mask(struct drm_device *dev, u32 *mask)
-{
- struct pci_dev *root;
- u32 lnkcap, lnkcap2;
-
- *mask = 0;
- if (!dev->pdev)
- return -EINVAL;
-
- root = dev->pdev->bus->self;
-
- /* we've been informed via and serverworks don't make the cut */
- if (root->vendor == PCI_VENDOR_ID_VIA ||
- root->vendor == PCI_VENDOR_ID_SERVERWORKS)
- return -EINVAL;
-
- pcie_capability_read_dword(root, PCI_EXP_LNKCAP, &lnkcap);
- pcie_capability_read_dword(root, PCI_EXP_LNKCAP2, &lnkcap2);
-
- if (lnkcap2) { /* PCIe r3.0-compliant */
- if (lnkcap2 & PCI_EXP_LNKCAP2_SLS_2_5GB)
- *mask |= DRM_PCIE_SPEED_25;
- if (lnkcap2 & PCI_EXP_LNKCAP2_SLS_5_0GB)
- *mask |= DRM_PCIE_SPEED_50;
- if (lnkcap2 & PCI_EXP_LNKCAP2_SLS_8_0GB)
- *mask |= DRM_PCIE_SPEED_80;
- } else { /* pre-r3.0 */
- if (lnkcap & PCI_EXP_LNKCAP_SLS_2_5GB)
- *mask |= DRM_PCIE_SPEED_25;
- if (lnkcap & PCI_EXP_LNKCAP_SLS_5_0GB)
- *mask |= (DRM_PCIE_SPEED_25 | DRM_PCIE_SPEED_50);
- }
-
- DRM_INFO("probing gen 2 caps for device %x:%x = %x/%x\n", root->vendor, root->device, lnkcap, lnkcap2);
- return 0;
-}
-EXPORT_SYMBOL(drm_pcie_get_speed_cap_mask);
-
#else
int drm_pci_init(struct drm_driver *driver, struct pci_driver *pdriver)
diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c
index 305a657..6ba204d 100644
--- a/drivers/gpu/drm/radeon/evergreen.c
+++ b/drivers/gpu/drm/radeon/evergreen.c
@@ -24,6 +24,7 @@
#include <linux/firmware.h>
#include <linux/platform_device.h>
#include <linux/slab.h>
+#include <linux/pci.h>
#include <drm/drmP.h>
#include "radeon.h"
#include "radeon_asic.h"
@@ -3871,11 +3872,11 @@ void evergreen_pcie_gen2_enable(struct radeon_device *rdev)
if (ASIC_IS_X2(rdev))
return;
- ret = drm_pcie_get_speed_cap_mask(rdev->ddev, &mask);
+ ret = pcie_get_speed_cap_mask(rdev->ddev->pdev, &mask);
if (ret != 0)
return;
- if (!(mask & DRM_PCIE_SPEED_50))
+ if (!(mask & PCIE_SPEED_50))
return;
speed_cntl = RREG32_PCIE_P(PCIE_LC_SPEED_CNTL);
diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c
index 0740db3..89a7387 100644
--- a/drivers/gpu/drm/radeon/r600.c
+++ b/drivers/gpu/drm/radeon/r600.c
@@ -30,6 +30,7 @@
#include <linux/firmware.h>
#include <linux/platform_device.h>
#include <linux/module.h>
+#include <linux/pci.h>
#include <drm/drmP.h>
#include <drm/radeon_drm.h>
#include "radeon.h"
@@ -4371,11 +4372,11 @@ static void r600_pcie_gen2_enable(struct radeon_device *rdev)
if (rdev->family <= CHIP_R600)
return;
- ret = drm_pcie_get_speed_cap_mask(rdev->ddev, &mask);
+ ret = pcie_get_speed_cap_mask(rdev->ddev->pdev, &mask);
if (ret != 0)
return;
- if (!(mask & DRM_PCIE_SPEED_50))
+ if (!(mask & PCIE_SPEED_50))
return;
speed_cntl = RREG32_PCIE_P(PCIE_LC_SPEED_CNTL);
diff --git a/drivers/gpu/drm/radeon/rv770.c b/drivers/gpu/drm/radeon/rv770.c
index d63fe1d..81c7f1c 100644
--- a/drivers/gpu/drm/radeon/rv770.c
+++ b/drivers/gpu/drm/radeon/rv770.c
@@ -28,6 +28,7 @@
#include <linux/firmware.h>
#include <linux/platform_device.h>
#include <linux/slab.h>
+#include <linux/pci.h>
#include <drm/drmP.h>
#include "radeon.h"
#include "radeon_asic.h"
@@ -1254,11 +1255,11 @@ static void rv770_pcie_gen2_enable(struct radeon_device *rdev)
if (ASIC_IS_X2(rdev))
return;
- ret = drm_pcie_get_speed_cap_mask(rdev->ddev, &mask);
+ ret = pcie_get_speed_cap_mask(rdev->ddev->pdev, &mask);
if (ret != 0)
return;
- if (!(mask & DRM_PCIE_SPEED_50))
+ if (!(mask & PCIE_SPEED_50))
return;
DRM_INFO("enabling PCIE gen 2 link speeds, disable with radeon.pcie_gen2=0\n");
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 2d94d74..39b2872 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1788,12 +1788,6 @@ extern int drm_get_pci_dev(struct pci_dev *pdev,
const struct pci_device_id *ent,
struct drm_driver *driver);
-#define DRM_PCIE_SPEED_25 1
-#define DRM_PCIE_SPEED_50 2
-#define DRM_PCIE_SPEED_80 4
-
-extern int drm_pcie_get_speed_cap_mask(struct drm_device *dev, u32 *speed_mask);
-
/* platform section */
extern int drm_platform_init(struct drm_driver *driver, struct platform_device *platform_device);
extern void drm_platform_exit(struct drm_driver *driver, struct platform_device *platform_device);
--
1.7.4.4
^ permalink raw reply related
* [PATCHv2 0/3] PCI Speed Cap Fixes for ppc64
From: Lucas Kannebley Tavares @ 2013-03-20 5:24 UTC (permalink / raw)
To: linux-pci, linuxppc-dev, dri-devel
Cc: David Airlie, Brian King, Thadeu Cascardo,
Lucas Kannebley Tavares, Bjorn Helgaas, Alex Deucher
This patch series first implements a function called pcie_get_speed_cap_mask
in the PCI subsystem based off from drm_pcie_get_speed_cap_mask in drm. Then
it removes the latter and fixes all references to it. And ultimately, it
implements an architecture-specific version of the same function for ppc64.
The refactor is done because the function that was used by drm is more
architecture goo than module-specific. Whilst the function also needed a
platform-specific implementation to get PCIE Gen2 speeds on ppc64.
Lucas Kannebley Tavares (3):
pci: added pcie_get_speed_cap_mask function
drm: removed drm_pcie_get_speed_cap_mask function
ppc64: implemented pcibios_get_speed_cap_mask
arch/powerpc/platforms/pseries/pci.c | 35 +++++++++++++++++++++++++++
drivers/gpu/drm/drm_pci.c | 38 -----------------------------
drivers/gpu/drm/radeon/evergreen.c | 5 ++-
drivers/gpu/drm/radeon/r600.c | 5 ++-
drivers/gpu/drm/radeon/rv770.c | 5 ++-
drivers/pci/pci.c | 44 ++++++++++++++++++++++++++++++++++
include/drm/drmP.h | 6 ----
include/linux/pci.h | 6 ++++
8 files changed, 94 insertions(+), 50 deletions(-)
--
1.7.4.4
^ permalink raw reply
* [PATCHv2 1/3] pci: added pcie_get_speed_cap_mask function
From: Lucas Kannebley Tavares @ 2013-03-20 5:24 UTC (permalink / raw)
To: linux-pci, linuxppc-dev, dri-devel
Cc: David Airlie, Brian King, Thadeu Cascardo,
Lucas Kannebley Tavares, Bjorn Helgaas, Alex Deucher
In-Reply-To: <1363757079-23550-1-git-send-email-lucaskt@linux.vnet.ibm.com>
Added function to gather the speed cap for a device and return a mask to
supported speeds. The function is divided into an interface and a weak
implementation so that architecture-specific functions can be called.
This is the first step in moving function drm_pcie_get_speed_cap_mask
from the drm subsystem to the pci one.
Signed-off-by: Lucas Kannebley Tavares <lucaskt@linux.vnet.ibm.com>
---
drivers/pci/pci.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
include/linux/pci.h | 6 ++++++
2 files changed, 50 insertions(+), 0 deletions(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index b099e00..d94ab79 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3931,6 +3931,50 @@ static int __init pci_setup(char *str)
}
early_param("pci", pci_setup);
+int __weak pcibios_get_speed_cap_mask(struct pci_dev *dev, u32 *mask)
+{
+ struct pci_dev *root;
+ u32 lnkcap, lnkcap2;
+
+ *mask = 0;
+ if (!dev)
+ return -EINVAL;
+
+ root = dev->bus->self;
+
+ /* we've been informed via and serverworks don't make the cut */
+ if (root->vendor == PCI_VENDOR_ID_VIA ||
+ root->vendor == PCI_VENDOR_ID_SERVERWORKS)
+ return -EINVAL;
+
+ pcie_capability_read_dword(root, PCI_EXP_LNKCAP, &lnkcap);
+ pcie_capability_read_dword(root, PCI_EXP_LNKCAP2, &lnkcap2);
+
+ if (lnkcap2) { /* PCIe r3.0-compliant */
+ if (lnkcap2 & PCI_EXP_LNKCAP2_SLS_2_5GB)
+ *mask |= PCIE_SPEED_25;
+ if (lnkcap2 & PCI_EXP_LNKCAP2_SLS_5_0GB)
+ *mask |= PCIE_SPEED_50;
+ if (lnkcap2 & PCI_EXP_LNKCAP2_SLS_8_0GB)
+ *mask |= PCIE_SPEED_80;
+ } else { /* pre-r3.0 */
+ if (lnkcap & PCI_EXP_LNKCAP_SLS_2_5GB)
+ *mask |= PCIE_SPEED_25;
+ if (lnkcap & PCI_EXP_LNKCAP_SLS_5_0GB)
+ *mask |= (PCIE_SPEED_25 | PCIE_SPEED_50);
+ }
+
+ dev_info(&dev->dev, "probing gen 2 caps for device %x:%x = %x/%x\n",
+ root->vendor, root->device, lnkcap, lnkcap2);
+ return 0;
+}
+
+int pcie_get_speed_cap_mask(struct pci_dev *dev, u32 *mask)
+{
+ return pcibios_get_speed_cap_mask(dev, mask);
+}
+EXPORT_SYMBOL(pcie_get_speed_cap_mask);
+
EXPORT_SYMBOL(pci_reenable_device);
EXPORT_SYMBOL(pci_enable_device_io);
EXPORT_SYMBOL(pci_enable_device_mem);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 2461033a..24a2f63 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1861,4 +1861,10 @@ static inline struct eeh_dev *pci_dev_to_eeh_dev(struct pci_dev *pdev)
*/
struct pci_dev *pci_find_upstream_pcie_bridge(struct pci_dev *pdev);
+#define PCIE_SPEED_25 1
+#define PCIE_SPEED_50 2
+#define PCIE_SPEED_80 4
+
+extern int pcie_get_speed_cap_mask(struct pci_dev *dev, u32 *speed_mask);
+
#endif /* LINUX_PCI_H */
--
1.7.4.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox