* Re: Please revert 928bea964827d7824b548c1f8e06eccbbc4d0d7d
From: Linus Torvalds @ 2013-09-27 17:10 UTC (permalink / raw)
To: Yinghai Lu
Cc: Bjorn Helgaas, linuxppc-dev, Linux Kernel list,
linux-pci@vger.kernel.org
In-Reply-To: <CAE9FiQVkW7d7vFx_kPJG-FEvO71SZjZmpR+dnwVm61wkYK=0YA@mail.gmail.com>
On Fri, Sep 27, 2013 at 9:01 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>
> So i would like to use the first way that you suggest : call pci_set_master
> PCIe port driver.
So I have to say, that if we can fix this with just adding a single
new pci_set_master() call, we should do that before we decide to
revert.
If other, bigger issues then come up, we can decide to revert. But if
there's a one-liner fix, let's just do that first, ok?
Mind sending a patch?
Linus
^ permalink raw reply
* Re: [PATCH] powerpc/mpic: Disable preemption when calling mpic_processor_id()
From: Kumar Gala @ 2013-09-27 17:09 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <1380298512.24959.384.camel@snotra.buserror.net>
On Sep 27, 2013, at 11:15 AM, Scott Wood wrote:
> On Fri, 2013-09-27 at 10:52 -0500, Kumar Gala wrote:
>> On Sep 26, 2013, at 7:18 PM, Scott Wood wrote:
>>=20
>>> Otherwise, we get a debug traceback due to the use of
>>> smp_processor_id() (or get_paca()) inside hard_smp_processor_id().
>>> mpic_host_map() is just looking for a default CPU, so it doesn't =
matter
>>> if we migrate after getting the CPU ID.
>>>=20
>>> Signed-off-by: Scott Wood <scottwood@freescale.com>
>>> ---
>>> arch/powerpc/sysdev/mpic.c | 8 +++++++-
>>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>>=20
>>> diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
>>> index 1be54fa..bdcb858 100644
>>> --- a/arch/powerpc/sysdev/mpic.c
>>> +++ b/arch/powerpc/sysdev/mpic.c
>>> @@ -1088,8 +1088,14 @@ static int mpic_host_map(struct irq_domain =
*h, unsigned int virq,
>>> * is done here.
>>> */
>>> if (!mpic_is_ipi(mpic, hw) && (mpic->flags & MPIC_NO_RESET)) {
>>> + int cpu;
>>> +
>>> + preempt_disable();
>>> + cpu =3D mpic_processor_id(mpic);
>>> + preempt_enable();
>>> +
>>=20
>> Any reason you didn't stick this inside of mpic_processor_id() ?
>=20
> Because the debug check might be valid for other callers and we don't
> want to defeat it. In this caller it's used only as a heuristic and
> thus it doesn't matter if we re-enable preemption before using the
> result.
Gotcha, I think you should reword the commit message. Reading it makes =
me think that preemption should be disabled for all mpic_processor_id() =
calls.
- k=
^ permalink raw reply
* Re: [PATCH V4 3/3] powerpc/85xx: Add TWR-P1025 board support
From: Scott Wood @ 2013-09-27 17:03 UTC (permalink / raw)
To: Xie Xiaobo-R63061
Cc: Wood Scott-B07421, linuxppc-dev@lists.ozlabs.org,
Johnston Michael-R49610
In-Reply-To: <69EC9ED88E3CC04094A78F8074A7986D5FF91C@039-SN1MPN1-003.039d.mgd.msft.net>
On Wed, 2013-09-25 at 04:50 -0500, Xie Xiaobo-R63061 wrote:
> > > +#if defined(CONFIG_SERIAL_QE)
> > > + /* On P1025TWR board, the UCC7 acted as UART port.
> > > + * However, The UCC7's CTS pin is low level in default,
> > > + * it will impact the transmission in full duplex
> > > + * communication. So disable the Flow control pin PA18.
> > > + * The UCC7 UART just can use RXD and TXD pins.
> > > + */
> > > + par_io_config_pin(0, 18, 0, 0, 0, 0); #endif
> >
> > Any reason not to do this unconditionally?
>
> This is a board issue, the code already have a condition - defined
> SERIAL_QE, and I will add a condition "if (machine_is(twr_p1025))".
My point was, is there any harm in doing this without regard to
CONFIG_SERIAL_QE?
-Scott
^ permalink raw reply
* Re: [PATCH V5 1/2] powerpc/85xx: Add QE common init function
From: Scott Wood @ 2013-09-27 17:00 UTC (permalink / raw)
To: Xie Xiaobo; +Cc: linuxppc-dev
In-Reply-To: <1380188253-4710-1-git-send-email-X.Xie@freescale.com>
On Thu, 2013-09-26 at 17:37 +0800, Xie Xiaobo wrote:
> +#ifdef CONFIG_QUICC_ENGINE
> +void __init mpc85xx_qe_init(void)
> +{
> + struct device_node *np;
> +
> + np = of_find_compatible_node(NULL, NULL, "fsl,qe");
> + if (!np) {
> + np = of_find_node_by_name(NULL, "qe");
> + if (!np) {
> + pr_err("%s: Could not find Quicc Engine node\n",
> + __func__);
> + return;
> + }
> + }
> +
> + qe_reset();
> + of_node_put(np);
You're missing the of_device_is_available() check:
> - np = of_find_compatible_node(NULL, NULL, "fsl,qe");
> - if (!np) {
> - np = of_find_node_by_name(NULL, "qe");
> - if (!np)
> - return;
> - }
> -
> - if (!of_device_is_available(np)) {
> - of_node_put(np);
> - return;
> - }
> -
> - qe_reset();
> - of_node_put(np);
-Scott
^ permalink raw reply
* Re: [PATCH 2/2] pci: fsl: rework PCI driver compatible with Layerscape
From: Scott Wood @ 2013-09-27 16:54 UTC (permalink / raw)
To: Minghuan Lian; +Cc: Bjorn Helgaas, linux-pci, linuxppc-dev, Zang Roy-R61911
In-Reply-To: <1379502122-20792-2-git-send-email-Minghuan.Lian@freescale.com>
On Wed, 2013-09-18 at 19:02 +0800, Minghuan Lian wrote:
> @@ -592,6 +719,7 @@ int fsl_pci_mcheck_exception(struct pt_regs *regs)
> #if defined(CONFIG_FSL_SOC_BOOKE) || defined(CONFIG_PPC_86xx)
>
> struct device_node *fsl_pci_primary;
> +extern const struct of_device_id fsl_pci_ids[];
Externs go in headers.
> -static int fsl_pci_probe(struct platform_device *pdev)
> +static int __init fsl_pci_probe(struct platform_device *pdev)
> {
> int ret;
> - struct device_node *node;
> + struct fsl_pci *pci;
> +
> + if (!of_device_is_available(pdev->dev.of_node)) {
> + dev_warn(&pdev->dev, "disabled\n");
> + return -ENODEV;
> + }
This should be dev_dbg().
> -#ifdef CONFIG_PM
> -static int fsl_pci_resume(struct device *dev)
> +static int __exit fsl_pci_remove(struct platform_device *pdev)
Why __exit? What happens if someone unbinds the PCI controller via
sysfs?
> +/*
> + * Structure of a PCI controller (host bridge)
> + */
> +struct fsl_pci {
> + struct list_head node;
> + int is_pcie;
bool is_pcie;
> +/* Return link status 0-> link, 1-> no link */
> +int fsl_pci_check_link(struct fsl_pci *pci);
bool
> +
> +/*
> + * The fsl_arch_* functions are arch hooks. Those functions are
> + * implemented as weak symbols so that they can be overridden by
> + * architecture specific code if needed.
> + */
> +
> +/* Return PCI64 DMA offset */
> +u64 fsl_arch_pci64_dma_offset(void);
Is this always guaranteed to exist?
> +/* Register PCI/PCIe controller to architecture system */
> +int __weak fsl_arch_pci_sys_register(struct fsl_pci *pci);
> +
> +/* Remove PCI/PCIe controller from architecture system */
> +void __weak fsl_arch_pci_sys_remove(struct fsl_pci *pci);
Why do these need to be weak? Won't there be exactly one implementation
per supported arch?
-Scott
^ permalink raw reply
* Re: [PATCH 2/2] pci: fsl: rework PCI driver compatible with Layerscape
From: Scott Wood @ 2013-09-27 16:47 UTC (permalink / raw)
To: Lian Minghuan-b31939
Cc: linux-pci, Zang Roy-R61911, Minghuan Lian, Bjorn Helgaas,
linuxppc-dev
In-Reply-To: <52456C0C.2020504@freescale.com>
On Fri, 2013-09-27 at 19:29 +0800, Lian Minghuan-b31939 wrote:
> Hi All,
>
> Can anyone comment on my code or help to pick up?
Please break it up into multiple patches, each with one logical change
that is individually explained. It will be much easier to review that
way.
-Scott
^ permalink raw reply
* Re: [PATCH] powerpc/mpic: Disable preemption when calling mpic_processor_id()
From: Scott Wood @ 2013-09-27 16:15 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev
In-Reply-To: <37986E99-BC99-4B67-9327-36CABA8E1A04@kernel.crashing.org>
On Fri, 2013-09-27 at 10:52 -0500, Kumar Gala wrote:
> On Sep 26, 2013, at 7:18 PM, Scott Wood wrote:
>
> > Otherwise, we get a debug traceback due to the use of
> > smp_processor_id() (or get_paca()) inside hard_smp_processor_id().
> > mpic_host_map() is just looking for a default CPU, so it doesn't matter
> > if we migrate after getting the CPU ID.
> >
> > Signed-off-by: Scott Wood <scottwood@freescale.com>
> > ---
> > arch/powerpc/sysdev/mpic.c | 8 +++++++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
> > index 1be54fa..bdcb858 100644
> > --- a/arch/powerpc/sysdev/mpic.c
> > +++ b/arch/powerpc/sysdev/mpic.c
> > @@ -1088,8 +1088,14 @@ static int mpic_host_map(struct irq_domain *h, unsigned int virq,
> > * is done here.
> > */
> > if (!mpic_is_ipi(mpic, hw) && (mpic->flags & MPIC_NO_RESET)) {
> > + int cpu;
> > +
> > + preempt_disable();
> > + cpu = mpic_processor_id(mpic);
> > + preempt_enable();
> > +
>
> Any reason you didn't stick this inside of mpic_processor_id() ?
Because the debug check might be valid for other callers and we don't
want to defeat it. In this caller it's used only as a heuristic and
thus it doesn't matter if we re-enable preemption before using the
result.
-Scott
^ permalink raw reply
* Re: Please revert 928bea964827d7824b548c1f8e06eccbbc4d0d7d
From: Yinghai Lu @ 2013-09-27 16:01 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Bjorn Helgaas, linux-pci@vger.kernel.org, Linus Torvalds,
linuxppc-dev, Linux Kernel list
In-Reply-To: <1380270519.27811.10.camel@pasglop>
On Fri, Sep 27, 2013 at 1:28 AM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> Hi Linus, Yinghai !
>
> Please consider reverting:
>
> 928bea964827d7824b548c1f8e06eccbbc4d0d7d
> PCI: Delay enabling bridges until they're needed
>
> (I'd suggest to revert now and maybe merge a better patch later)
>
> This breaks PCI on the PowerPC "powernv" platform (which is booted via
> kexec) and probably x86 as well under similar circumstances. It will
> basically break PCIe if the bus master bit of the bridge isn't set at
> boot (by the firmware for example, or because kexec'ing cleared it).
>
> The reason is that the PCIe port driver will call pci_enable_device() on
> the bridge (on everything under the sun actually), which will marked the
> device enabled (but will not do a set_master).
>
> Because of that, pci_enable_bridge() later on (called as a result of the
> child device driver doing pci_enable_device) will see the bridge as
> already enabled and will not call pci_set_master() on it.
>
> Now, this could probably be fixed by simply doing pci_set_master() in
> the PCIe port driver, but I find the whole logic very fragile (anything
> that "enables" the bridge without setting master or for some reason
> clears master will forever fail to re-enable it).
>
> Maybe a better option is to unconditionally do pci_set_mater() in
> pci_enable_bridge(), ie, move the call to before the enabled test.
>
> However I am not too happy with that either. My experience with bridges
> is that if bus master isn't set, they will also fail to report AER
> errors and other similar upstream transactions. We might want to get
> these reported properly even if no downstream device got successfully
> enabled.
>
> So I think the premises of the patches are flawed, at least on PCI
> express, and we should stick to always enabling bridges (at least the
> bus master bit on them).
there is pci_set_master everywhere in pci drivers.
So i would like to use the first way that you suggest : call pci_set_master
PCIe port driver.
Thanks
Yinghai
^ permalink raw reply
* Re: therm_pm72 units, interface
From: Michel Dänzer @ 2013-09-27 15:44 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: Ben Hutchings, linuxppc-dev
In-Reply-To: <1375699987.12557.46.camel@pasglop>
On Mon, 2013-08-05 at 20:53 +1000, Benjamin Herrenschmidt wrote:
> On Mon, 2013-08-05 at 12:32 +0200, Michel Dänzer wrote:
> >
> > I did that, sorry should have mentioned that.
> >
> >
> > > > @@ -468,5 +478,3 @@ static struct platform_driver
> > i2c_powermac_driver
> > > > =
> > > > {
> > > > };
> > > >
> > > > module_platform_driver(i2c_powermac_driver);
> > > > -
> > > > -MODULE_ALIAS("platform:i2c-powermac");
> > >
> > > Maybe add the module alias back ? It shouldn't be necessary...
> >
> > Doesn't help.
>
> Hrm, that might require some more involved debugging, figuring out
> what's up with udev etc... that or maybe bisecting.
>
> From what I can tell, we do attach an OF node to the platform device,
> but since the driver has no of match table, we should still fallback to
> the old platform matching style.
>
> I think I see... adding Grant.
Did that work? He's not in Cc on the post I received from the list, but
that might be due to his mailman settings.
> Grant, something broke the auto-loading the of i2c-powermac module. It's
> a platform device, but while it does have a populated "of_node, its
> driver doesn't have an OF match table and relies on the old style
> platform device matching.
>
> That's broken with newer kernels, platform_uevent() calls
> of_device_uevent_modalias() which sees the of_node and thus doesn't
> return -ENOMEM, and we don't create a platform modalias anymore.
>
> Is it legit to add several MODALIAS ? If yes we could add both ... if
> not, we have a problem. Doing real OF matching with that critter is
> tricky at best for various reasons but it needs the of_node because it
> uses it to scan for children.
>
> Any suggestion ?
Any news on this? I recently learned the hard way that loading
i2c-powermac manually via /etc/modules isn't a good workaround, as that
only happens after fsck...
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
^ permalink raw reply
* Re: [PATCH] powerpc/mpic: Disable preemption when calling mpic_processor_id()
From: Kumar Gala @ 2013-09-27 15:52 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <1380241098-7561-1-git-send-email-scottwood@freescale.com>
On Sep 26, 2013, at 7:18 PM, Scott Wood wrote:
> Otherwise, we get a debug traceback due to the use of
> smp_processor_id() (or get_paca()) inside hard_smp_processor_id().
> mpic_host_map() is just looking for a default CPU, so it doesn't =
matter
> if we migrate after getting the CPU ID.
>=20
> Signed-off-by: Scott Wood <scottwood@freescale.com>
> ---
> arch/powerpc/sysdev/mpic.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>=20
> diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
> index 1be54fa..bdcb858 100644
> --- a/arch/powerpc/sysdev/mpic.c
> +++ b/arch/powerpc/sysdev/mpic.c
> @@ -1088,8 +1088,14 @@ static int mpic_host_map(struct irq_domain *h, =
unsigned int virq,
> * is done here.
> */
> if (!mpic_is_ipi(mpic, hw) && (mpic->flags & MPIC_NO_RESET)) {
> + int cpu;
> +
> + preempt_disable();
> + cpu =3D mpic_processor_id(mpic);
> + preempt_enable();
> +
Any reason you didn't stick this inside of mpic_processor_id() ?
> mpic_set_vector(virq, hw);
> - mpic_set_destination(virq, mpic_processor_id(mpic));
> + mpic_set_destination(virq, cpu);
> mpic_irq_set_priority(virq, 8);
> }
>=20
> --=20
> 1.8.1.2
>=20
>=20
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply
* Re: mm: insure topdown mmap chooses addresses above security minimum
From: Timothy Pepper @ 2013-09-27 15:39 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-mips, linux-sh, linux-mm, Paul Mackerras, H. Peter Anvin,
sparclinux, Michel Lespinasse, Russell King, x86, Ingo Molnar,
Rik van Riel, Al Viro, James Morris, Thomas Gleixner,
linux-arm-kernel, linuxppc-dev, Ralf Baechle, Paul Mundt,
Andrew Morton, Linus Torvalds, David S. Miller
In-Reply-To: <20130925174436.GA14037@gmail.com>
On Wed 25 Sep at 19:44:36 +0200 mingo@kernel.org said:
>
> * Timothy Pepper <timothy.c.pepper@linux.intel.com> wrote:
>
> > On Wed 25 Sep at 09:30:49 +0200 mingo@kernel.org said:
> > > > info.flags = VM_UNMAPPED_AREA_TOPDOWN;
> > > > info.length = len;
> > > > - info.low_limit = PAGE_SIZE;
> > > > + info.low_limit = max(PAGE_SIZE, PAGE_ALIGN(mmap_min_addr));
> > > > info.high_limit = mm->mmap_base;
> > > > info.align_mask = filp ? get_align_mask() : 0;
> > > > info.align_offset = pgoff << PAGE_SHIFT;
> > >
> > > There appears to be a lot of repetition in these methods - instead of
> > > changing 6 places it would be more future-proof to first factor out the
> > > common bits and then to apply the fix to the shared implementation.
> >
> > Besides that existing redundancy in the multiple somewhat similar
> > arch_get_unmapped_area_topdown() functions, I was expecting people might
> > question the added redundancy of the six instances of:
> >
> > max(PAGE_SIZE, PAGE_ALIGN(mmap_min_addr));
>
> That redundancy would be automatically addressed by my suggestion.
Yes.
I'm looking at the cleanup and will post a bisectable series that
introduces a common helper, addes the calls to use that helper where
applicable (looks like it might be a few dozen per arch locations), and
then the single line change for the topdown case within the common helper
to do:
info->low_limit = max(PAGE_SIZE, PAGE_ALIGN(mmap_min_addr));
--
Tim Pepper <timothy.c.pepper@linux.intel.com>
Intel Open Source Technology Center
^ permalink raw reply
* [PATCH v1] powerpc/mpc512x: silence build warning upon disabled DIU
From: Gerhard Sittig @ 2013-09-27 15:28 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Gerhard Sittig, Anatolij Gustschin
a disabled Kconfig option results in a reference to a not implemented
routine when the IS_ENABLED() macro is used for both conditional
implementation of the routine as well as a C language source code test
at the call site -- the "if (0) func();" construct only gets eliminated
later by the optimizer, while the compiler already has emitted its
warning about "func()" being undeclared
provide an empty implementation for the mpc512x_setup_diu() and
mpc512x_init_diu() routines in case of the disabled option, to avoid the
compiler warning which is considered fatal and breaks compilation
the bug appeared with commit 2abbbb63c90ab55ca3f054772c2e5ba7df810c48
"powerpc/mpc512x: move common code to shared.c file", how to reproduce:
make mpc512x_defconfig
echo CONFIG_FB_FSL_DIU=n >> .config && make olddefconfig
make
CC arch/powerpc/platforms/512x/mpc512x_shared.o
.../arch/powerpc/platforms/512x/mpc512x_shared.c: In function 'mpc512x_init_early':
.../arch/powerpc/platforms/512x/mpc512x_shared.c:456:3: error: implicit declaration of function 'mpc512x_init_diu' [-Werror=implicit-function-declaration]
.../arch/powerpc/platforms/512x/mpc512x_shared.c: In function 'mpc512x_setup_arch':
.../arch/powerpc/platforms/512x/mpc512x_shared.c:469:3: error: implicit declaration of function 'mpc512x_setup_diu' [-Werror=implicit-function-declaration]
cc1: all warnings being treated as errors
make[4]: *** [arch/powerpc/platforms/512x/mpc512x_shared.o] Error 1
Signed-off-by: Gerhard Sittig <gsi@denx.de>
CC: <stable@vger.kernel.org> # v3.11
---
arch/powerpc/platforms/512x/mpc512x_shared.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/powerpc/platforms/512x/mpc512x_shared.c b/arch/powerpc/platforms/512x/mpc512x_shared.c
index a82a41b..1a7b1d0 100644
--- a/arch/powerpc/platforms/512x/mpc512x_shared.c
+++ b/arch/powerpc/platforms/512x/mpc512x_shared.c
@@ -303,6 +303,9 @@ void __init mpc512x_setup_diu(void)
diu_ops.release_bootmem = mpc512x_release_bootmem;
}
+#else
+void __init mpc512x_setup_diu(void) { /* EMPTY */ }
+void __init mpc512x_init_diu(void) { /* EMPTY */ }
#endif
void __init mpc512x_init_IRQ(void)
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH v3] powerpc/83xx: gianfar_ptp: select 1588 clock source through dts file
From: Kumar Gala @ 2013-09-27 15:28 UTC (permalink / raw)
To: Aida Mynzhasova; +Cc: devicetree, richardcochran, linuxppc-dev, netdev
In-Reply-To: <1380289227-12029-1-git-send-email-aida.mynzhasova@skitlab.ru>
On Sep 27, 2013, at 8:40 AM, Aida Mynzhasova wrote:
> Currently IEEE 1588 timer reference clock source is determined through
> hard-coded value in gianfar_ptp driver. This patch allows to select =
ptp
> clock source by means of device tree file node.
>=20
> For instance:
>=20
> fsl,cksel =3D <0>;
>=20
> for using external (TSEC_TMR_CLK input) high precision timer
> reference clock.
>=20
> Other acceptable values:
>=20
> <1> : eTSEC system clock
> <2> : eTSEC1 transmit clock
> <3> : RTC clock input
>=20
> When this attribute isn't used, eTSEC system clock will serve as
> IEEE 1588 timer reference clock.
>=20
> Signed-off-by: Aida Mynzhasova <aida.mynzhasova@skitlab.ru>
> ---
> Documentation/devicetree/bindings/net/fsl-tsec-phy.txt | 18 =
+++++++++++++++++-
> drivers/net/ethernet/freescale/gianfar_ptp.c | 4 +++-
> 2 files changed, 20 insertions(+), 2 deletions(-)
Acked-by: Kumar Gala <galak@codeaurora.org>
- k=
^ permalink raw reply
* [PATCH] Correct memory hotplug for ppc with sparse vmemmap
From: Nathan Fontenot @ 2013-09-27 15:18 UTC (permalink / raw)
To: linuxppc-dev
Previous commit 46723bfa540... introduced a new config option
HAVE_BOOTMEM_INFO_NODE that ended up breaking memory hot-remove for ppc
when sparse vmemmap is not defined.
This patch defines HAVE_BOOTMEM_INFO_NODE for ppc and adds the call to
register_page_bootmem_info_node. Without this we get a BUG_ON for memory
hot remove in put_page_bootmem().
This also adds a stub for register_page_bootmem_memmap to allow ppc to build
with sparse vmemmap defined. Leaving this as a stub is fine since the same
vmemmap addresses are also handled in vmemmap_populate and as such are
properly mapped.
Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
---
arch/powerpc/mm/init_64.c | 4 ++++
arch/powerpc/mm/mem.c | 9 +++++++++
mm/Kconfig | 2 +-
3 files changed, 14 insertions(+), 1 deletion(-)
Index: powerpc/arch/powerpc/mm/init_64.c
===================================================================
--- powerpc.orig/arch/powerpc/mm/init_64.c
+++ powerpc/arch/powerpc/mm/init_64.c
@@ -300,5 +300,9 @@ void vmemmap_free(unsigned long start, u
{
}
+void register_page_bootmem_memmap(unsigned long section_nr,
+ struct page *start_page, unsigned long size)
+{
+}
#endif /* CONFIG_SPARSEMEM_VMEMMAP */
Index: powerpc/arch/powerpc/mm/mem.c
===================================================================
--- powerpc.orig/arch/powerpc/mm/mem.c
+++ powerpc/arch/powerpc/mm/mem.c
@@ -297,12 +297,21 @@ void __init paging_init(void)
}
#endif /* ! CONFIG_NEED_MULTIPLE_NODES */
+static void __init register_page_bootmem_info(void)
+{
+ int i;
+
+ for_each_online_node(i)
+ register_page_bootmem_info_node(NODE_DATA(i));
+}
+
void __init mem_init(void)
{
#ifdef CONFIG_SWIOTLB
swiotlb_init(0);
#endif
+ register_page_bootmem_info();
high_memory = (void *) __va(max_low_pfn * PAGE_SIZE);
set_max_mapnr(max_pfn);
free_all_bootmem();
Index: powerpc/mm/Kconfig
===================================================================
--- powerpc.orig/mm/Kconfig
+++ powerpc/mm/Kconfig
@@ -183,7 +183,7 @@ config MEMORY_HOTPLUG_SPARSE
config MEMORY_HOTREMOVE
bool "Allow for memory hot remove"
select MEMORY_ISOLATION
- select HAVE_BOOTMEM_INFO_NODE if X86_64
+ select HAVE_BOOTMEM_INFO_NODE if (X86_64 || PPC64)
depends on MEMORY_HOTPLUG && ARCH_ENABLE_MEMORY_HOTREMOVE
depends on MIGRATION
^ permalink raw reply
* Re: [RFC PATCH 09/11] kvm: simplify processor compat check
From: Paolo Bonzini @ 2013-09-27 15:14 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: Gleb Natapov, <kvm@vger.kernel.org> list, Alexander Graf,
kvm-ppc, Paul Mackerras, linuxppc-dev
In-Reply-To: <87r4ca9zmi.fsf@linux.vnet.ibm.com>
Il 27/09/2013 15:13, Aneesh Kumar K.V ha scritto:
> Alexander Graf <agraf@suse.de> writes:
>
>> On 27.09.2013, at 12:03, Aneesh Kumar K.V wrote:
>>
>>> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>>
>> Missing patch description.
>>
>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>>
>> I fail to see how this really simplifies things, but at the end of the
>> day it's Gleb's and Paolo's call.
>
> will do. It avoid calling
>
> for_each_online_cpu(cpu) {
> smp_call_function_single()
>
> on multiple architecture.
I agree with Alex.
The current code is not specially awesome; having
kvm_arch_check_processor_compat take an int* disguised as a void* is a
bit ugly indeed.
However, the API makes sense and tells you that it is being passed as a
callback (to smp_call_function_single in this case).
You are making the API more complicated to use on the arch layer
(because arch maintainers now have to think "do I need to check this on
all online CPUs?") and making the "leaf" POWER code less legible because
it still has the weird void()(void *) calling convention.
If anything, you could change kvm_arch_check_processor_compat to return
an int and accept no argument, and introduce a wrapper that kvm_init
passes to smp_call_function_single.
Paolo
> We also want to make the smp call function a callback of opaque. Hence
> this should be made arch specific.
>
> int kvm_arch_check_processor_compat(void *opaque)
> {
> int r,cpu;
> struct kvmppc_ops *kvm_ops = (struct kvmppc_ops *)opaque;
> for_each_online_cpu(cpu) {
> smp_call_function_single(cpu,
> kvm_ops->check_processor_compat,
> &r, 1);
> if (r < 0)
> break;
> }
> return r;
> }
>
> against
>
> - for_each_online_cpu(cpu) {
> - smp_call_function_single(cpu,
> - kvm_arch_check_processor_compat,
> - &r, 1);
> - if (r < 0)
> - goto out_free_1;
> - }
> +
> + r = kvm_arch_check_processor_compat(opaque);
> + if (r < 0)
> + goto out_free_1;
>
>
>
>>
>> Which brings me to the next issue: You forgot to CC kvm@vger on your
>> patch set. Gleb and Paolo don't read kvm-ppc@vger. And they shouldn't
>> have to. Every kvm patch that you want review on or that should get
>> applied needs to be sent to kvm@vger. If you want to tag it as PPC
>> specific patch, do so by CC'ing kvm-ppc@vger.
>
> Will do in the next update
>
> -aneesh
>
^ permalink raw reply
* Re: [PATCH V2] powerpc/kvm/book3s_hv: propagate H_SET_MODE_RESOURCE_LE to the host
From: Greg Kurz @ 2013-09-27 14:45 UTC (permalink / raw)
To: Laurent Dufour
Cc: linuxppc-dev, Rusty Russell, Paul Mackerras, Anton Blanchard
In-Reply-To: <20130927135930.10288.86526.stgit@nimbus>
On Fri, 27 Sep 2013 15:59:30 +0200
Laurent Dufour <ldufour@linux.vnet.ibm.com> wrote:
> Follow-up to Anton's H_SET_MODE patch, the host should be taken aware
> of guest endianess change.
>
> The hcall H_SET_MODE/H_SET_MODE_RESOURCE_LE is processed in kvm and
> then propagated to the host.
>
Even if it seems a bit odd to get H_SET_MODE handled both by kvm and
qemu, it is a simple way to get the job done. Unless we expect tons of
calls to H_SET_MODE_RESOURCE_LE to occur, I do not see a better way for
the host code to know the guest endianess.
FYI, with this patch, Rusty's (Cc'ed) virtio endianess patchset for
qemu works like a charm: my guest kernel calls h_set_mode once at boot
time, qemu gets notified and keeps the information. Do we need more ?
> v2: taking in account the Paul Mackerras's comment, using H_TOO_HARD
> to propagate only H_SET_MODE_RESOURCE_LE to the host.
>
> Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
> ---
Tested-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> arch/powerpc/kvm/book3s_hv.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv.c
> b/arch/powerpc/kvm/book3s_hv.c index 998cad3..be0af39 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -523,14 +523,14 @@ static int kvmppc_h_set_mode(struct kvm_vcpu
> *vcpu, unsigned long mflags, kvm_for_each_vcpu(n, v, kvm)
> v->arch.intr_msr &= ~MSR_LE;
> kick_all_cpus_sync();
> - return H_SUCCESS;
> + return H_TOO_HARD; /* propagating to the
> host */
>
> case 1:
> kvm->arch.lpcr |= LPCR_ILE;
> kvm_for_each_vcpu(n, v, kvm)
> v->arch.intr_msr |= MSR_LE;
> kick_all_cpus_sync();
> - return H_SUCCESS;
> + return H_TOO_HARD; /* propagating to the
> host */
>
> default:
> return H_UNSUPPORTED_FLAG_START;
> @@ -599,6 +599,8 @@ int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu)
> kvmppc_get_gpr(vcpu, 5),
> kvmppc_get_gpr(vcpu, 6),
> kvmppc_get_gpr(vcpu, 7));
> + if (ret == H_TOO_HARD)
> + return RESUME_HOST;
> break;
>
> case H_XIRR:
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
--
Gregory Kurz kurzgreg@fr.ibm.com
gkurz@linux.vnet.ibm.com
Software Engineer @ IBM/Meiosys http://www.ibm.com
Tel +33 (0)562 165 496
"Anarchy is about taking complete responsibility for yourself."
Alan Moore.
^ permalink raw reply
* Re: [PATCH 3/3] KVM: PPC: Book3S: Add support for hwrng found on some powernv systems
From: Anshuman Khandual @ 2013-09-27 14:15 UTC (permalink / raw)
To: Michael Ellerman
Cc: tytso, herbert, gleb, agraf, kvm-ppc, linux-kernel, linuxppc-dev,
Paul Mackerras, kvm, mpm, pbonzini
In-Reply-To: <1380177066-3835-3-git-send-email-michael@ellerman.id.au>
On 09/26/2013 12:01 PM, Michael Ellerman wrote:
> +int powernv_hwrng_present(void)
> +{
> + return __raw_get_cpu_var(powernv_rng) != NULL;
> +}
> +
> static unsigned long rng_whiten(struct powernv_rng *rng, unsigned long val)
> {
> unsigned long parity;
> @@ -42,6 +48,17 @@ static unsigned long rng_whiten(struct powernv_rng *rng, unsigned long val)
> return val;
> }
>
> +int powernv_get_random_real_mode(unsigned long *v)
> +{
> + struct powernv_rng *rng;
> +
> + rng = __raw_get_cpu_var(powernv_rng);
> +
> + *v = rng_whiten(rng, in_rm64(rng->regs_real));
> +
Will it be in_be64() instead of in_rm64() ? Its failing the build here. Except this
all individual patches build correctly.
Regards
Anshuman
^ permalink raw reply
* [PATCH V2] powerpc/kvm/book3s_hv: propagate H_SET_MODE_RESOURCE_LE to the host
From: Laurent Dufour @ 2013-09-27 13:59 UTC (permalink / raw)
To: Paul Mackerras, linuxppc-dev; +Cc: Anton Blanchard
In-Reply-To: <20130925223118.GA2844@iris.ozlabs.ibm.com>
Follow-up to Anton's H_SET_MODE patch, the host should be taken aware of
guest endianess change.
The hcall H_SET_MODE/H_SET_MODE_RESOURCE_LE is processed in kvm and then
propagated to the host.
v2: taking in account the Paul Mackerras's comment, using H_TOO_HARD to
propagate only H_SET_MODE_RESOURCE_LE to the host.
Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
---
arch/powerpc/kvm/book3s_hv.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 998cad3..be0af39 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -523,14 +523,14 @@ static int kvmppc_h_set_mode(struct kvm_vcpu *vcpu, unsigned long mflags,
kvm_for_each_vcpu(n, v, kvm)
v->arch.intr_msr &= ~MSR_LE;
kick_all_cpus_sync();
- return H_SUCCESS;
+ return H_TOO_HARD; /* propagating to the host */
case 1:
kvm->arch.lpcr |= LPCR_ILE;
kvm_for_each_vcpu(n, v, kvm)
v->arch.intr_msr |= MSR_LE;
kick_all_cpus_sync();
- return H_SUCCESS;
+ return H_TOO_HARD; /* propagating to the host */
default:
return H_UNSUPPORTED_FLAG_START;
@@ -599,6 +599,8 @@ int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu)
kvmppc_get_gpr(vcpu, 5),
kvmppc_get_gpr(vcpu, 6),
kvmppc_get_gpr(vcpu, 7));
+ if (ret == H_TOO_HARD)
+ return RESUME_HOST;
break;
case H_XIRR:
^ permalink raw reply related
* [PATCH v3] powerpc/83xx: gianfar_ptp: select 1588 clock source through dts file
From: Aida Mynzhasova @ 2013-09-27 13:40 UTC (permalink / raw)
To: linuxppc-dev; +Cc: devicetree, richardcochran, netdev
Currently IEEE 1588 timer reference clock source is determined through
hard-coded value in gianfar_ptp driver. This patch allows to select ptp
clock source by means of device tree file node.
For instance:
fsl,cksel = <0>;
for using external (TSEC_TMR_CLK input) high precision timer
reference clock.
Other acceptable values:
<1> : eTSEC system clock
<2> : eTSEC1 transmit clock
<3> : RTC clock input
When this attribute isn't used, eTSEC system clock will serve as
IEEE 1588 timer reference clock.
Signed-off-by: Aida Mynzhasova <aida.mynzhasova@skitlab.ru>
---
Documentation/devicetree/bindings/net/fsl-tsec-phy.txt | 18 +++++++++++++++++-
drivers/net/ethernet/freescale/gianfar_ptp.c | 4 +++-
2 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt b/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
index 2c6be03..d2ea460 100644
--- a/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
+++ b/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
@@ -86,6 +86,7 @@ General Properties:
Clock Properties:
+ - fsl,cksel Timer reference clock source.
- fsl,tclk-period Timer reference clock period in nanoseconds.
- fsl,tmr-prsc Prescaler, divides the output clock.
- fsl,tmr-add Frequency compensation value.
@@ -97,7 +98,7 @@ Clock Properties:
clock. You must choose these carefully for the clock to work right.
Here is how to figure good values:
- TimerOsc = system clock MHz
+ TimerOsc = selected reference clock MHz
tclk_period = desired clock period nanoseconds
NominalFreq = 1000 / tclk_period MHz
FreqDivRatio = TimerOsc / NominalFreq (must be greater that 1.0)
@@ -114,6 +115,20 @@ Clock Properties:
Pulse Per Second (PPS) signal, since this will be offered to the PPS
subsystem to synchronize the Linux clock.
+ Reference clock source is determined by the value, which is holded
+ in CKSEL bits in TMR_CTRL register. "fsl,cksel" property keeps the
+ value, which will be directly written in those bits, that is why,
+ according to reference manual, the next clock sources can be used:
+
+ <0> - external high precision timer reference clock (TSEC_TMR_CLK
+ input is used for this purpose);
+ <1> - eTSEC system clock;
+ <2> - eTSEC1 transmit clock;
+ <3> - RTC clock input.
+
+ When this attribute is not used, eTSEC system clock will serve as
+ IEEE 1588 timer reference clock.
+
Example:
ptp_clock@24E00 {
@@ -121,6 +136,7 @@ Example:
reg = <0x24E00 0xB0>;
interrupts = <12 0x8 13 0x8>;
interrupt-parent = < &ipic >;
+ fsl,cksel = <1>;
fsl,tclk-period = <10>;
fsl,tmr-prsc = <100>;
fsl,tmr-add = <0x999999A4>;
diff --git a/drivers/net/ethernet/freescale/gianfar_ptp.c b/drivers/net/ethernet/freescale/gianfar_ptp.c
index 098f133..e006a09 100644
--- a/drivers/net/ethernet/freescale/gianfar_ptp.c
+++ b/drivers/net/ethernet/freescale/gianfar_ptp.c
@@ -452,7 +452,9 @@ static int gianfar_ptp_probe(struct platform_device *dev)
err = -ENODEV;
etsects->caps = ptp_gianfar_caps;
- etsects->cksel = DEFAULT_CKSEL;
+
+ if (get_of_u32(node, "fsl,cksel", &etsects->cksel))
+ etsects->cksel = DEFAULT_CKSEL;
if (get_of_u32(node, "fsl,tclk-period", &etsects->tclk_period) ||
get_of_u32(node, "fsl,tmr-prsc", &etsects->tmr_prsc) ||
--
1.8.1.2
^ permalink raw reply related
* Re: [RFC PATCH 09/11] kvm: simplify processor compat check
From: Aneesh Kumar K.V @ 2013-09-27 13:13 UTC (permalink / raw)
To: Alexander Graf
Cc: <kvm@vger.kernel.org> list, Gleb Natapov, kvm-ppc,
Paul Mackerras, Paolo Bonzini, linuxppc-dev
In-Reply-To: <E5A81672-864D-4E33-B15A-5F6927CC7C13@suse.de>
Alexander Graf <agraf@suse.de> writes:
> On 27.09.2013, at 12:03, Aneesh Kumar K.V wrote:
>
>> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>
> Missing patch description.
>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>
> I fail to see how this really simplifies things, but at the end of the
> day it's Gleb's and Paolo's call.
will do. It avoid calling
for_each_online_cpu(cpu) {
smp_call_function_single()
on multiple architecture.
We also want to make the smp call function a callback of opaque. Hence
this should be made arch specific.
int kvm_arch_check_processor_compat(void *opaque)
{
int r,cpu;
struct kvmppc_ops *kvm_ops = (struct kvmppc_ops *)opaque;
for_each_online_cpu(cpu) {
smp_call_function_single(cpu,
kvm_ops->check_processor_compat,
&r, 1);
if (r < 0)
break;
}
return r;
}
against
- for_each_online_cpu(cpu) {
- smp_call_function_single(cpu,
- kvm_arch_check_processor_compat,
- &r, 1);
- if (r < 0)
- goto out_free_1;
- }
+
+ r = kvm_arch_check_processor_compat(opaque);
+ if (r < 0)
+ goto out_free_1;
>
> Which brings me to the next issue: You forgot to CC kvm@vger on your
> patch set. Gleb and Paolo don't read kvm-ppc@vger. And they shouldn't
> have to. Every kvm patch that you want review on or that should get
> applied needs to be sent to kvm@vger. If you want to tag it as PPC
> specific patch, do so by CC'ing kvm-ppc@vger.
Will do in the next update
-aneesh
^ permalink raw reply
* Re: [RFC PATCH 08/11] kvm: powerpc: book3s: Support building HV and PR KVM as module
From: Aneesh Kumar K.V @ 2013-09-27 13:08 UTC (permalink / raw)
To: Alexander Graf; +Cc: paulus, linuxppc-dev, kvm-ppc
In-Reply-To: <32B8402B-25D1-41F4-A3FB-B945B72EDC4C@suse.de>
Alexander Graf <agraf@suse.de> writes:
> On 27.09.2013, at 12:03, Aneesh Kumar K.V wrote:
>
>> diff --git a/arch/powerpc/kvm/book3s_64_mmu_host.c b/arch/powerpc/kvm/book3s_64_mmu_host.c
>> index fd5b393..775d368 100644
>> --- a/arch/powerpc/kvm/book3s_64_mmu_host.c
>> +++ b/arch/powerpc/kvm/book3s_64_mmu_host.c
>> @@ -27,6 +27,7 @@
>> #include <asm/machdep.h>
>> #include <asm/mmu_context.h>
>> #include <asm/hw_irq.h>
>> +
>
> Stray whitespace change
>
will fix
>> #include "trace_pr.h"
>>
>> #define PTE_SIZE 12
>> diff --git a/arch/powerpc/kvm/book3s_emulate.c b/arch/powerpc/kvm/book3s_emulate.c
>> index b9841ad..20d03c2 100644
>> --- a/arch/powerpc/kvm/book3s_emulate.c
>> +++ b/arch/powerpc/kvm/book3s_emulate.c
>> @@ -172,7 +172,7 @@ int kvmppc_core_emulate_op_pr(struct kvm_run *run, struct kvm_vcpu *vcpu,
>> vcpu->arch.mmu.tlbie(vcpu, addr, large);
>> break;
>> }
>> -#ifdef CONFIG_KVM_BOOK3S_64_PR
>> +#ifdef CONFIG_KVM_BOOK3S_PR
>
> Why?
If i have CONFIG_KVM_BOOK3S_64_PR=m #ifdef CONFIG_KVM_BOOK3S_64_PR will
not work. There is a runtime check I can use IS_ENABLED(). But didn't
want to do those. Hence moved to the symbol which will be set as
CONFIG_KVM_BOOK3S_PR = y
-aneesh
^ permalink raw reply
* Re: [RFC PATCH 07/11] kvm: powerpc: book3s: pr: move PR related tracepoints to a separate header
From: Aneesh Kumar K.V @ 2013-09-27 13:06 UTC (permalink / raw)
To: Alexander Graf; +Cc: paulus, linuxppc-dev, kvm-ppc
In-Reply-To: <C02177CF-C8A0-4E84-82FE-1994742B8EBD@suse.de>
Alexander Graf <agraf@suse.de> writes:
> On 27.09.2013, at 12:03, Aneesh Kumar K.V wrote:
>
>> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>>
>> This patch moves PR related tracepoints to a separate header. This
>> enables in converting PR to a kernel module which will be done in
>> later patches
>>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>> ---
>> arch/powerpc/kvm/book3s_64_mmu_host.c | 2 +-
>> arch/powerpc/kvm/book3s_mmu_hpte.c | 2 +-
>> arch/powerpc/kvm/book3s_pr.c | 3 +-
>> arch/powerpc/kvm/trace.h | 234 +--------------------------
>> arch/powerpc/kvm/trace_pr.h | 297 ++++++++++++++++++++++++++++++++++
>> 5 files changed, 308 insertions(+), 230 deletions(-)
>> create mode 100644 arch/powerpc/kvm/trace_pr.h
>>
>> diff --git a/arch/powerpc/kvm/book3s_64_mmu_host.c b/arch/powerpc/kvm/book3s_64_mmu_host.c
>> index 329a978..fd5b393 100644
>> --- a/arch/powerpc/kvm/book3s_64_mmu_host.c
>> +++ b/arch/powerpc/kvm/book3s_64_mmu_host.c
>> @@ -27,7 +27,7 @@
>> #include <asm/machdep.h>
>> #include <asm/mmu_context.h>
>> #include <asm/hw_irq.h>
>> -#include "trace.h"
>> +#include "trace_pr.h"
>>
>> #define PTE_SIZE 12
>>
>> diff --git a/arch/powerpc/kvm/book3s_mmu_hpte.c b/arch/powerpc/kvm/book3s_mmu_hpte.c
>> index d2d280b..4556168 100644
>> --- a/arch/powerpc/kvm/book3s_mmu_hpte.c
>> +++ b/arch/powerpc/kvm/book3s_mmu_hpte.c
>> @@ -28,7 +28,7 @@
>> #include <asm/mmu_context.h>
>> #include <asm/hw_irq.h>
>>
>> -#include "trace.h"
>> +#include "trace_pr.h"
>>
>> #define PTE_SIZE 12
>>
>> diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
>> index 2a97279..99d0839 100644
>> --- a/arch/powerpc/kvm/book3s_pr.c
>> +++ b/arch/powerpc/kvm/book3s_pr.c
>> @@ -41,7 +41,8 @@
>> #include <linux/vmalloc.h>
>> #include <linux/highmem.h>
>>
>> -#include "trace.h"
>> +#define CREATE_TRACE_POINTS
>> +#include "trace_pr.h"
>>
>> /* #define EXIT_DEBUG */
>> /* #define DEBUG_EXT */
>> diff --git a/arch/powerpc/kvm/trace.h b/arch/powerpc/kvm/trace.h
>> index a088e9a..7d5a136 100644
>> --- a/arch/powerpc/kvm/trace.h
>> +++ b/arch/powerpc/kvm/trace.h
>> @@ -85,6 +85,12 @@ TRACE_EVENT(kvm_ppc_instr,
>> {41, "HV_PRIV"}
>> #endif
>>
>> +#ifndef CONFIG_KVM_BOOK3S_PR
>> +/*
>> + * For pr we define this in trace_pr.h since it pr can be built as
>> + * a module
>
> Not sure I understand the need. If the config option is available, so
> should the struct field. Worst case that happens with HV is that we
> get empty shadow_srr1 values in our trace, no?
That is not the real reason. trace.h get built as part of kvm.ko or as
part of kernel. These trace functions actually get called from
kvm-pr.ko. To make they build i would either need EXPORT_SYMBOL or move
the definition of them to kvm-pr.ko. I did the later and moved only pr
related traces to kvm-pr.ko
>
> If your goal is to make it more obvious whether we are tracing in PR
> or HV land (which is a reasonable goal), then you should also split
> off all non-common trace points into a special hv trace header so that
> it's obvious whether we are looking at HV or PR.
-aneesh
^ permalink raw reply
* Re: [RFC PATCH 06/11] kvm: powerpc: book3s: Add is_hv_enabled to kvmppc_ops
From: Aneesh Kumar K.V @ 2013-09-27 13:03 UTC (permalink / raw)
To: Alexander Graf; +Cc: paulus, linuxppc-dev, kvm-ppc
In-Reply-To: <9BAA779D-FE0C-4971-992B-57D80F945906@suse.de>
Alexander Graf <agraf@suse.de> writes:
> On 27.09.2013, at 12:03, Aneesh Kumar K.V wrote:
>
>> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>>
>> This help us to identify whether we are running with hypervisor mode KVM
>> enabled. The change is needed so that we can have both HV and PR kvm
>> enabled in the same kernel.
>>
>> If both HV and PR KVM are included, interrupts come in to the HV version
>> of the kvmppc_interrupt code, which then jumps to the PR handler,
>> renamed to kvmppc_interrupt_pr, if the guest is a PR guest.
>>
>> Allowing both PR and HV in the same kernel required some changes to
>> kvm_dev_ioctl_check_extension(), since the values returned now can't
>> be selected with #ifdefs as much as previously. We look at is_hv_enabled
>> to return the right value when checking for capabilities.For capabilities that
>> are only provided by HV KVM, we return the HV value only if
>> is_hv_enabled is true. For capabilities provided by PR KVM but not HV,
>> we return the PR value only if is_hv_enabled is false.
>>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>> ---
>> arch/powerpc/include/asm/kvm_book3s.h | 53 --------------------------------
>> arch/powerpc/include/asm/kvm_ppc.h | 5 +--
>> arch/powerpc/kvm/Makefile | 2 +-
>> arch/powerpc/kvm/book3s.c | 44 +++++++++++++++++++++++++++
>> arch/powerpc/kvm/book3s_hv.c | 1 +
>> arch/powerpc/kvm/book3s_hv_rmhandlers.S | 4 +++
>> arch/powerpc/kvm/book3s_pr.c | 1 +
>> arch/powerpc/kvm/book3s_segment.S | 7 ++++-
>> arch/powerpc/kvm/book3s_xics.c | 2 +-
>> arch/powerpc/kvm/powerpc.c | 54 ++++++++++++++++++---------------
>> 10 files changed, 90 insertions(+), 83 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h
>> index 3efba3c..ba33c49 100644
>> --- a/arch/powerpc/include/asm/kvm_book3s.h
>> +++ b/arch/powerpc/include/asm/kvm_book3s.h
>> @@ -297,59 +297,6 @@ static inline ulong kvmppc_get_fault_dar(struct kvm_vcpu *vcpu)
>> return vcpu->arch.fault_dar;
>> }
>>
>> -#ifdef CONFIG_KVM_BOOK3S_PR
>> -
>> -static inline unsigned long kvmppc_interrupt_offset(struct kvm_vcpu *vcpu)
>> -{
>> - return to_book3s(vcpu)->hior;
>> -}
>> -
>> -static inline void kvmppc_update_int_pending(struct kvm_vcpu *vcpu,
>> - unsigned long pending_now, unsigned long old_pending)
>> -{
>> - if (pending_now)
>> - vcpu->arch.shared->int_pending = 1;
>> - else if (old_pending)
>> - vcpu->arch.shared->int_pending = 0;
>> -}
>> -
>> -static inline bool kvmppc_critical_section(struct kvm_vcpu *vcpu)
>> -{
>> - ulong crit_raw = vcpu->arch.shared->critical;
>> - ulong crit_r1 = kvmppc_get_gpr(vcpu, 1);
>> - bool crit;
>> -
>> - /* Truncate crit indicators in 32 bit mode */
>> - if (!(vcpu->arch.shared->msr & MSR_SF)) {
>> - crit_raw &= 0xffffffff;
>> - crit_r1 &= 0xffffffff;
>> - }
>> -
>> - /* Critical section when crit == r1 */
>> - crit = (crit_raw == crit_r1);
>> - /* ... and we're in supervisor mode */
>> - crit = crit && !(vcpu->arch.shared->msr & MSR_PR);
>> -
>> - return crit;
>> -}
>> -#else /* CONFIG_KVM_BOOK3S_PR */
>> -
>> -static inline unsigned long kvmppc_interrupt_offset(struct kvm_vcpu *vcpu)
>> -{
>> - return 0;
>> -}
>> -
>> -static inline void kvmppc_update_int_pending(struct kvm_vcpu *vcpu,
>> - unsigned long pending_now, unsigned long old_pending)
>> -{
>> -}
>> -
>> -static inline bool kvmppc_critical_section(struct kvm_vcpu *vcpu)
>> -{
>> - return false;
>> -}
>> -#endif
>> -
>> /* Magic register values loaded into r3 and r4 before the 'sc' assembly
>> * instruction for the OSI hypercalls */
>> #define OSI_SC_MAGIC_R3 0x113724FA
>> diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
>> index 4d9641c..58e732f 100644
>> --- a/arch/powerpc/include/asm/kvm_ppc.h
>> +++ b/arch/powerpc/include/asm/kvm_ppc.h
>> @@ -169,6 +169,7 @@ extern int kvmppc_xics_int_on(struct kvm *kvm, u32 irq);
>> extern int kvmppc_xics_int_off(struct kvm *kvm, u32 irq);
>>
>> struct kvmppc_ops {
>> + bool is_hv_enabled;
>
> This doesn't really belong into an ops struct. Either you compare
>
> if (kvmppc_ops == kvmppc_ops_pr)
will do that in the next update.
>
> against a global known good ops struct or you put the hint somewhere into the kvm struct.
>
>> int (*get_sregs)(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs);
>> int (*set_sregs)(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs);
>> int (*get_one_reg)(struct kvm_vcpu *vcpu, u64 id,
>> @@ -309,10 +310,10 @@ static inline void kvmppc_set_xics_phys(int cpu, unsigned long addr)
>>
>> static inline u32 kvmppc_get_xics_latch(void)
>> {
>> - u32 xirr = get_paca()->kvm_hstate.saved_xirr;
>> + u32 xirr;
>>
>> + xirr = get_paca()->kvm_hstate.saved_xirr;
>> get_paca()->kvm_hstate.saved_xirr = 0;
>> -
>
> I don't see any functionality change here?
>
>> return xirr;
Mistake on my side, I had a if condition in there before, which i later
removed. But forgot to move the assignment back. Will fix.
>> }
>>
>> diff --git a/arch/powerpc/kvm/Makefile b/arch/powerpc/kvm/Makefile
>> index c343793..a514ecd 100644
>> --- a/arch/powerpc/kvm/Makefile
>> +++ b/arch/powerpc/kvm/Makefile
>> @@ -77,7 +77,7 @@ kvm-book3s_64-builtin-objs-$(CONFIG_KVM_BOOK3S_64_HANDLER) += \
>> book3s_rmhandlers.o
>> endif
>>
>> -kvm-book3s_64-objs-$(CONFIG_KVM_BOOK3S_64_HV) := \
>> +kvm-book3s_64-objs-$(CONFIG_KVM_BOOK3S_64_HV) += \
>
> This change looks unrelated?
>
yes. Will move it to the correct patch
>> book3s_hv.o \
>> book3s_hv_interrupts.o \
>> book3s_64_mmu_hv.o
>> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
>> index bdc3f95..12f94bf 100644
>> --- a/arch/powerpc/kvm/book3s.c
>> +++ b/arch/powerpc/kvm/book3s.c
>> @@ -69,6 +69,50 @@ void kvmppc_core_load_guest_debugstate(struct kvm_vcpu *vcpu)
>> {
>> }
>>
>> +static inline unsigned long kvmppc_interrupt_offset(struct kvm_vcpu *vcpu)
>
> Please drop the "inline" keyword on functions in .c files :). That way
> the compiler tells us about unused functions.
ok .
>
>> +{
>> + if (!kvmppc_ops->is_hv_enabled)
>> + return to_book3s(vcpu)->hior;
>> + return 0;
>> +}
>> +
>> +static inline void kvmppc_update_int_pending(struct kvm_vcpu *vcpu,
>> + unsigned long pending_now, unsigned long old_pending)
>> +{
>> + if (kvmppc_ops->is_hv_enabled)
>> + return;
>> + if (pending_now)
>> + vcpu->arch.shared->int_pending = 1;
>> + else if (old_pending)
>> + vcpu->arch.shared->int_pending = 0;
>> +}
>> +
....
>> diff --git a/arch/powerpc/kvm/book3s_segment.S b/arch/powerpc/kvm/book3s_segment.S
>> index 1abe478..e0229dd 100644
>> --- a/arch/powerpc/kvm/book3s_segment.S
>> +++ b/arch/powerpc/kvm/book3s_segment.S
>> @@ -161,9 +161,14 @@ kvmppc_handler_trampoline_enter_end:
>> .global kvmppc_handler_trampoline_exit
>> kvmppc_handler_trampoline_exit:
>>
>> +#if defined(CONFIG_KVM_BOOK3S_HV)
>> +.global kvmppc_interrupt_pr
>> +kvmppc_interrupt_pr:
>> + ld r9, HSTATE_SCRATCH2(r13)
>> +#else
>> .global kvmppc_interrupt
>> kvmppc_interrupt:
>
> Just always call it kvmppc_interrupt_pr and thus share at least that
> part of the code :).
But if i don't have HV enabled, we don't compile book3s_hv_rmhandlers.S
Hence don't have the kvmppc_interrupt symbol defined.
>
>> -
>> +#endif
>> /* Register usage at this point:
>> *
>> * SPRG_SCRATCH0 = guest R13
>> diff --git a/arch/powerpc/kvm/book3s_xics.c b/arch/powerpc/kvm/book3s_xics.c
>> index f0c732e..fa7625d 100644
>> --- a/arch/powerpc/kvm/book3s_xics.c
>> +++ b/arch/powerpc/kvm/book3s_xics.c
>> @@ -818,7 +818,7 @@ int kvmppc_xics_hcall(struct kvm_vcpu *vcpu, u32 req)
>> }
>>
>> /* Check for real mode returning too hard */
>> - if (xics->real_mode)
>> + if (xics->real_mode && kvmppc_ops->is_hv_enabled)
>> return kvmppc_xics_rm_complete(vcpu, req);
>>
>> switch (req) {
>> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
>> index 69b9305..00a96fc 100644
>> --- a/arch/powerpc/kvm/powerpc.c
>> +++ b/arch/powerpc/kvm/powerpc.c
>> @@ -52,7 +52,6 @@ int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu)
>> return 1;
>> }
>>
>> -#ifndef CONFIG_KVM_BOOK3S_64_HV
>> /*
>> * Common checks before entering the guest world. Call with interrupts
>> * disabled.
>> @@ -127,7 +126,6 @@ int kvmppc_prepare_to_enter(struct kvm_vcpu *vcpu)
>>
>> return r;
>> }
>> -#endif /* CONFIG_KVM_BOOK3S_64_HV */
>>
>> int kvmppc_kvm_pv(struct kvm_vcpu *vcpu)
>> {
>> @@ -194,11 +192,9 @@ int kvmppc_sanity_check(struct kvm_vcpu *vcpu)
>> if ((vcpu->arch.cpu_type != KVM_CPU_3S_64) && vcpu->arch.papr_enabled)
>> goto out;
>>
>> -#ifdef CONFIG_KVM_BOOK3S_64_HV
>> /* HV KVM can only do PAPR mode for now */
>> - if (!vcpu->arch.papr_enabled)
>> + if (!vcpu->arch.papr_enabled && kvmppc_ops->is_hv_enabled)
>> goto out;
>> -#endif
>>
>> #ifdef CONFIG_KVM_BOOKE_HV
>> if (!cpu_has_feature(CPU_FTR_EMB_HV))
>> @@ -322,22 +318,26 @@ int kvm_dev_ioctl_check_extension(long ext)
>> case KVM_CAP_DEVICE_CTRL:
>> r = 1;
>> break;
>> -#ifndef CONFIG_KVM_BOOK3S_64_HV
>> case KVM_CAP_PPC_PAIRED_SINGLES:
>> case KVM_CAP_PPC_OSI:
>> case KVM_CAP_PPC_GET_PVINFO:
>> #if defined(CONFIG_KVM_E500V2) || defined(CONFIG_KVM_E500MC)
>> case KVM_CAP_SW_TLB:
>> #endif
>> -#ifdef CONFIG_KVM_MPIC
>> - case KVM_CAP_IRQ_MPIC:
>> -#endif
>> - r = 1;
>> + /* We support this only for PR */
>> + r = !kvmppc_ops->is_hv_enabled;
>
> Reading this, have you test compiled your code against e500 configs?
Not yet.
>
>> break;
>> +#ifdef CONFIG_KVM_MMIO
>> case KVM_CAP_COALESCED_MMIO:
>> r = KVM_COALESCED_MMIO_PAGE_OFFSET;
>> break;
>> #endif
>> +#ifdef CONFIG_KVM_MPIC
>> + case KVM_CAP_IRQ_MPIC:
>> + r = 1;
>> + break;
>> +#endif
>> +
>> #ifdef CONFIG_PPC_BOOK3S_64
>> case KVM_CAP_SPAPR_TCE:
>> case KVM_CAP_PPC_ALLOC_HTAB:
>> @@ -348,32 +348,37 @@ int kvm_dev_ioctl_check_extension(long ext)
>> r = 1;
>> break;
>> #endif /* CONFIG_PPC_BOOK3S_64 */
>> -#ifdef CONFIG_KVM_BOOK3S_64_HV
>> +#ifdef CONFIG_KVM_BOOK3S_HV
>> case KVM_CAP_PPC_SMT:
>> - r = threads_per_core;
>> + if (kvmppc_ops->is_hv_enabled)
>> + r = threads_per_core;
>> + else
>> + r = 0;
>
> 0? Or 1?
>
That check extension was not supported before on PR. So not sure what
the value should be. May be 1 is better indicating we have one thread.
Will change.
-aneesh
^ permalink raw reply
* Re: [RFC PATCH 05/11] kvm: powerpc: book3s: Add kvmppc_ops callback for HV and PR specific operations
From: Aneesh Kumar K.V @ 2013-09-27 12:52 UTC (permalink / raw)
To: Alexander Graf; +Cc: paulus, linuxppc-dev, kvm-ppc
In-Reply-To: <8EBD07D5-C57D-460C-9392-4933A0E340FA@suse.de>
Alexander Graf <agraf@suse.de> writes:
> On 27.09.2013, at 12:03, Aneesh Kumar K.V wrote:
>
>> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>>
>> This moves HV and PR specific functions to kvmppc_ops callback.
>> This is needed so that we can enable HV and PR together in the
>> same kernel. Actual changes to enable both come in the later
>> patch.This also renames almost all of the symbols that exist in both PR and HV
>> KVM for clarity. Symbols in the PR KVM implementation get "_pr"
>> appended, and those in the HV KVM implementation get "_hv". Then,
>> in book3s.c, we add a function with the name without the suffix and
>> arrange for it to call the appropriate kvmppc_ops callback depending on
>> which kvm type we selected during VM creation.
>>
>> NOTE: we still don't enable selecting both the HV and PR together
>> in this commit that will be done by a later commit.
>>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>> ---
>> arch/powerpc/include/asm/kvm_book3s.h | 5 +-
>> arch/powerpc/include/asm/kvm_ppc.h | 63 ++++++++--
>> arch/powerpc/kvm/Kconfig | 15 ++-
>> arch/powerpc/kvm/Makefile | 9 +-
>> arch/powerpc/kvm/book3s.c | 145 +++++++++++++++++++++-
>> arch/powerpc/kvm/book3s_32_mmu_host.c | 2 +-
>> arch/powerpc/kvm/book3s_64_mmu_host.c | 2 +-
>> arch/powerpc/kvm/book3s_64_mmu_hv.c | 17 ++-
>> arch/powerpc/kvm/book3s_emulate.c | 8 +-
>> arch/powerpc/kvm/book3s_hv.c | 226 +++++++++++++++++++++++++---------
>> arch/powerpc/kvm/book3s_interrupts.S | 2 +-
>> arch/powerpc/kvm/book3s_pr.c | 196 ++++++++++++++++++-----------
>> arch/powerpc/kvm/emulate.c | 6 +-
>> arch/powerpc/kvm/powerpc.c | 58 +++------
>> 14 files changed, 539 insertions(+), 215 deletions(-)
>>
>>
>
> [...]
>
>> @@ -888,14 +890,8 @@ int kvmppc_set_one_reg(struct kvm_vcpu *vcpu, u64 id, union kvmppc_one_reg *val)
>> return r;
>> }
>>
>> -int kvmppc_core_check_processor_compat(void)
>> -{
>> - if (cpu_has_feature(CPU_FTR_HVMODE))
>> - return 0;
>> - return -EIO;
>> -}
>> -
>> -struct kvm_vcpu *kvmppc_core_vcpu_create(struct kvm *kvm, unsigned int id)
>> +static struct kvm_vcpu *kvmppc_core_vcpu_create_hv(struct kvm *kvm,
>> + unsigned int id)
>> {
>> struct kvm_vcpu *vcpu;
>> int err = -EINVAL;
>> @@ -920,7 +916,6 @@ struct kvm_vcpu *kvmppc_core_vcpu_create(struct kvm *kvm, unsigned int id)
>> vcpu->arch.ctrl = CTRL_RUNLATCH;
>> /* default to host PVR, since we can't spoof it */
>> vcpu->arch.pvr = mfspr(SPRN_PVR);
>> - kvmppc_set_pvr(vcpu, vcpu->arch.pvr);
>
> Where is this one going?
That is same as the line above.
void kvmppc_set_pvr_hv(struct kvm_vcpu *vcpu, u32 pvr)
{
vcpu->arch.pvr = pvr;
}
>
>> spin_lock_init(&vcpu->arch.vpa_update_lock);
>> spin_lock_init(&vcpu->arch.tbacct_lock);
>> vcpu->arch.busy_preempt = TB_NIL;
>> @@ -972,7 +967,7 @@ static void unpin_vpa(struct kvm *kvm, struct kvmppc_vpa *vpa)
>> vpa->dirty);
>> }
>>
>> -void kvmppc_core_vcpu_free(struct kvm_vcpu *vcpu)
>> +static void kvmppc_core_vcpu_free_hv(struct kvm_vcpu *vcpu)
>> {
>> spin_lock(&vcpu->arch.vpa_update_lock);
>> unpin_vpa(vcpu->kvm, &vcpu->arch.dtl);
>> @@ -983,6 +978,12 @@ void kvmppc_core_vcpu_free(struct kvm_vcpu *vcpu)
>> kmem_cache_free(kvm_vcpu_cache, vcpu);
>> }
>>
>> +static int kvmppc_core_check_requests_hv(struct kvm_vcpu *vcpu)
>> +{
>> + /* Indicate we want to get back into the guest */
>> + return 1;
>> +}
>> +
>>
>
> [...]
>
>> + case KVM_PPC_GET_HTAB_FD: {
>> + struct kvm_get_htab_fd ghf;
>> +
>> + r = -EFAULT;
>> + if (copy_from_user(&ghf, argp, sizeof(ghf)))
>> + break;
>> + r = kvm_vm_ioctl_get_htab_fd(kvm, &ghf);
>> + break;
>> + }
>> +
>> + default:
>> + r = -ENOTTY;
>> + }
>> +
>> + return r;
>> }
>>
>> -static int kvmppc_book3s_hv_init(void)
>> +/* FIXME!! move to header */
>
> Hrm :)
yes, want to get something out for review. Will fix if we agree on the
approach.
>
>> +extern void kvmppc_core_flush_memslot_hv(struct kvm *kvm,
>> + struct kvm_memory_slot *memslot);
>> +extern int kvm_unmap_hva_hv(struct kvm *kvm, unsigned long hva);
>> +extern int kvm_unmap_hva_range_hv(struct kvm *kvm, unsigned long start,
>> + unsigned long end);
>> +extern int kvm_age_hva_hv(struct kvm *kvm, unsigned long hva);
>> +extern int kvm_test_age_hva_hv(struct kvm *kvm, unsigned long hva);
>> +extern void kvm_set_spte_hva_hv(struct kvm *kvm, unsigned long hva, pte_t pte);
>> +
>> +static struct kvmppc_ops kvmppc_hv_ops = {
>> + .get_sregs = kvm_arch_vcpu_ioctl_get_sregs_hv,
>> + .set_sregs = kvm_arch_vcpu_ioctl_set_sregs_hv,
>> + .get_one_reg = kvmppc_get_one_reg_hv,
>> + .set_one_reg = kvmppc_set_one_reg_hv,
>> + .vcpu_load = kvmppc_core_vcpu_load_hv,
>> + .vcpu_put = kvmppc_core_vcpu_put_hv,
>> + .set_msr = kvmppc_set_msr_hv,
>> + .vcpu_run = kvmppc_vcpu_run_hv,
>> + .vcpu_create = kvmppc_core_vcpu_create_hv,
>> + .vcpu_free = kvmppc_core_vcpu_free_hv,
>> + .check_requests = kvmppc_core_check_requests_hv,
>> + .get_dirty_log = kvm_vm_ioctl_get_dirty_log_hv,
>> + .flush_memslot = kvmppc_core_flush_memslot_hv,
>> + .prepare_memory_region = kvmppc_core_prepare_memory_region_hv,
>> + .commit_memory_region = kvmppc_core_commit_memory_region_hv,
>> + .unmap_hva = kvm_unmap_hva_hv,
>> + .unmap_hva_range = kvm_unmap_hva_range_hv,
>> + .age_hva = kvm_age_hva_hv,
>> + .test_age_hva = kvm_test_age_hva_hv,
>> + .set_spte_hva = kvm_set_spte_hva_hv,
>> + .mmu_destroy = kvmppc_mmu_destroy_hv,
>> + .free_memslot = kvmppc_core_free_memslot_hv,
>> + .create_memslot = kvmppc_core_create_memslot_hv,
>> + .init_vm = kvmppc_core_init_vm_hv,
>> + .destroy_vm = kvmppc_core_destroy_vm_hv,
>> + .check_processor_compat = kvmppc_core_check_processor_compat_hv,
>> + .get_smmu_info = kvm_vm_ioctl_get_smmu_info_hv,
>> + .emulate_op = kvmppc_core_emulate_op_hv,
>> + .emulate_mtspr = kvmppc_core_emulate_mtspr_hv,
>> + .emulate_mfspr = kvmppc_core_emulate_mfspr_hv,
>> + .fast_vcpu_kick = kvmppc_fast_vcpu_kick_hv,
>> + .arch_vm_ioctl = kvm_arch_vm_ioctl_hv,
>> +};
>> +
>>
>
> [...]
>
>> @@ -1390,8 +1389,42 @@ out:
>> return r;
>> }
>>
>> +static void kvmppc_core_flush_memslot_pr(struct kvm *kvm,
>> + struct kvm_memory_slot *memslot)
>> +{
>> + return;
>> +}
>> +
>> +static int kvmppc_core_prepare_memory_region_pr(struct kvm *kvm,
>> + struct kvm_memory_slot *memslot,
>> + struct kvm_userspace_memory_region *mem)
>> +{
>> + return 0;
>> +}
>> +
>> +static void kvmppc_core_commit_memory_region_pr(struct kvm *kvm,
>> + struct kvm_userspace_memory_region *mem,
>> + const struct kvm_memory_slot *old)
>> +{
>> + return;
>> +}
>> +
>> +static void kvmppc_core_free_memslot_pr(struct kvm_memory_slot *free,
>> + struct kvm_memory_slot *dont)
>> +{
>> + return;
>> +}
>> +
>> +static int kvmppc_core_create_memslot_pr(struct kvm_memory_slot *slot,
>> + unsigned long npages)
>> +{
>> + return 0;
>> +}
>> +
>> +
>> #ifdef CONFIG_PPC64
>> -int kvm_vm_ioctl_get_smmu_info(struct kvm *kvm, struct kvm_ppc_smmu_info *info)
>> +static int kvm_vm_ioctl_get_smmu_info_pr(struct kvm *kvm,
>> + struct kvm_ppc_smmu_info *info)
>
> You're dereferencing this function unconditionally now, probably
> breaking book3s_32 along the way :).
will double check that.
>
> I'm not really happy with the naming scheme either, but I can't really
> think of anything better right now. In an ideal world all functions
> would still have the same names and we merely make them static and
> refer to them through structs :).
I was following rest of the kernel source there. For ex: struct
file_operations function pointers get pointed to by different fs
specific callback, they all have fs details in their name. I also found
that having _hv and _pr in the name allowed for easy grep and clarity
in what different files should contain.
-aneesh
^ permalink raw reply
* Re: [RFC PATCH 04/11] kvm: powerpc: book3s: Add a new config variable CONFIG_KVM_BOOK3S_HV
From: Aneesh Kumar K.V @ 2013-09-27 12:45 UTC (permalink / raw)
To: Alexander Graf; +Cc: paulus, linuxppc-dev, kvm-ppc
In-Reply-To: <F47EA67A-B8C0-409F-A4D4-3BA7D11E01F5@suse.de>
Alexander Graf <agraf@suse.de> writes:
> On 27.09.2013, at 12:03, Aneesh Kumar K.V wrote:
>
>> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>>
>> This help ups to select the relevant code in the kernel code
>> when we later move HV and PR bits as seperate modules.
>
> I don't think I grasp what semantically the difference between
> CONFIG_KVM_BOOK3S_HV and CONFIG_KVM_BOOK3S_64_HV is :).
When we later enable kvm-hv CONFIG_KVM_BOOK3S_64_HV becomes 'm' So we
can't do #ifdef CONFIG_KVM_BOOK3S_64_HV
-aneesh
^ permalink raw reply
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