* [linux-sunxi] [PATCH v6 0/5] drm: sun8i: Add DE2 HDMI video support
From: Ondřej Jirman @ 2016-11-21 18:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161121191416.706b80c25d476fa002a001b1@free.fr>
Dne 21.11.2016 v 19:14 Jean-Francois Moine napsal(a):
> On Mon, 21 Nov 2016 01:54:53 +0100
> Ond?ej Jirman <megous@megous.com> wrote:
>
>> Dne 20.11.2016 v 12:32 Jean-Francois Moine napsal(a):
>>> This patchset series adds HDMI video support to the Allwinner
>>> sun8i SoCs which include the display engine 2 (DE2).
>>> The driver contains the code for the A83T and H3, but it could be
>>> used/extended for other SoCs as the A64, H2 and H5.
>>
>> Hi,
>>
>> I'm trying to test your patches on Orange Pi PC, and I've run into a few
>> issues: (I'm using sunxi-ng with the same patches as last time, to make
>> it work with your driver)
>>
>> 1] I just get pink output on the monitor - there's some signal, but it's
>> pink (or more like magenta).
>>
>> dmesg ouput indicates no error:
>>
>> [ 1.887823] [drm] Initialized
>> [ 1.888503] sun8i-de2 1000000.de-controller: bound
>> 1c0c000.lcd-controller (ops 0xc0a63894)
>> [ 2.057298] sun8i-de2 1000000.de-controller: bound 1ee0000.hdmi (ops
>> 0xc0a63b54)
>> [ 2.057304] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
>> [ 2.057307] [drm] No driver support for vblank timestamp query.
>> [ 2.690862] Console: switching to colour frame buffer device 240x67
>> [ 2.723059] sun8i-de2 1000000.de-controller: fb0: frame buffer device
> [snip]
>
> My H3 boards work correctly, except the Orange PI 2 when it cannot read
> the EDID (but it is OK after reboot).
>
> Did you check if the EDID was correctly read?
EDID is correctly read (I verified that it is the same as with the v5
version of the driver), but there's one difference I noted. v5 says dpms
is Off, while v6 says dpms is On.
> Which resolution do you expect?
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161121/1453ca53/attachment.sig>
^ permalink raw reply
* [BUG] hdlcd gets confused about base address
From: Russell King - ARM Linux @ 2016-11-21 18:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161121182324.GP1005@e106497-lin.cambridge.arm.com>
On Mon, Nov 21, 2016 at 06:23:24PM +0000, Liviu Dudau wrote:
> On Mon, Nov 21, 2016 at 05:56:02PM +0000, Russell King - ARM Linux wrote:
> > For me, the image shift was 100% reproducable. With the above patch
> > and a call to drm_crtc_vblank_on() in the enable path, it seems to
> > behave correctly - I can alternately switch between 1920x1080 and
> > 1280x1024 and it behaves correctly. Indeed, my debug prints show that
> > the right thing is happening wrt disabling the controller:
>
> OK, so I'll take it that you did not also use your patch to fix the base
> plane calculations, or was that included as well in your stack?
It was before that patch - so it was using crtc_x and crtc_y. However,
I can guarantee that those were both zero (as I've previously
described.)
> > That's more of a generic DRM issue - the CRTC layer doesn't get a
> > look-in when a connector parses the modes supplied from the display,
> > so there's no real way for the CRTC layer to apply any kind of
> > limitations to the available modes, except when a mode is attempted
> > to be set.
> >
> > I don't want to see an "interlace" DT property introduced for the
> > TDA998x, because that's the wrong approach - it would be adding a
> > property for the needs of the implementation, and not a description
> > of the hardware.
>
> AFAICT the issue is the fact that while HDLCD could scan out the alternate
> lines with a bit of a convoluted hack, there is no way to tell TDA19988
> to generate the interlaced timings. And no, I'm not advocating introducing
> a DT property as this is a runtime mode, depending on the resolution
> selected by userspace.
The TDA998x doesn't "generate" the timings. They come from the input
to it, the TDA998x merely tracks where it is within the frame, so it
knows where it can place things like the infoframes and other data.
So, the responsibility for generating the interlaced timings is with
the CRTC.
That means the CRTC needs to not only scan out alternate lines (which
is the easy bit - setting the pitch to twice the value) but it also
needs to be able to adjust the timing of the vertical sync by half
a line. The HDLCD from what I can see does not support that, the
overall system does not support for interlaced modes.
> > Whether that has any bearing on the reproducability of this or not, I've
> > no idea.
>
> The one factor that could affect it is the capability of the SCP firmware
> to generate the exact pixel clock for your 1080p mode. If it doesn't, then
> restoring the old mode might lead to an incorrect synchronisation with the
> TDA chip. Current (less than 1.5 years old I guess) SCP firmware has that
> sorted via an hdlcd.dat file that pre-calculates a lot of common pixel clock
> frequencies).
The TDA998x takes the sync signals itself to synchronise with the CRTC,
and the pixel clock had better be synchronous with the data being closed
out of the CRTC otherwise its going to be in violation of the RGB data
setup and hold timings - which will cause random colour errors.
That isn't what's going on here - the image is rock stable, it's just
shifted.
I tried inverting the sync signals from the CRTC to the TDA998x, and
that shifts the display (as I expect, because the TDA998x synchronises
on the transition of the sync signals not on their absolute values) and
at that point I get the black sync bars appearing - again as expected.
Same kind of effect if I swap the horizontal front and back porches.
Of course, adjusting such things necessitates the TDA998x to re-lock
to the CRTC each time something like that changes, and the image
shift remains.
As I described originally, the _only_ two things that solved the image
shift was (a) shifting the framebuffer start address earlier than it
should be, or (b) disabling the CRTC and re-enabling the CRTC. Both
of those were tried using devmem2 in userspace with no patches to the
HDLCD code over v4.9-rc5.
The only patches that would be in effect are my TDA998x patch stack
(which you've already tested), the i2c-designware patches to sort that
crappy thing out, and a dirty patch to the TDA998x code to read the
EDID in 16 byte chunks [*], so that the i2c-designware crappage never
causes a problem.
* - I'm not submitting this patch, because while it may solve the
EDID reading issue on Juno, it's putting intimate knowledge of
i2c-designware into the TDA998x driver - it's a hack around the
problem, it's not a real fix. It's possible that there are other
i2c-designware crappages out there which have even smaller FIFOs
which would need us to read in even smaller chunks for reliability.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* [RFC PATCH v2 4/8] arm64: compat: Add a 32-bit vDSO
From: Catalin Marinas @ 2016-11-21 18:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <f5ff1b88-aba3-2c3a-f65f-66a2e00f4764@arm.com>
On Mon, Nov 21, 2016 at 03:45:55PM +0000, Kevin Brodsky wrote:
> On 04/11/16 20:03, Catalin Marinas wrote:
> >On Fri, Oct 28, 2016 at 11:20:07AM +0100, Kevin Brodsky wrote:
> >>On 28/10/16 04:09, Jisheng Zhang wrote:
> >>>On Thu, 27 Oct 2016 17:30:54 +0100 Kevin Brodsky wrote:
> >>>>+# Force -O2 to avoid libgcc dependencies
> >>>>+VDSO_CFLAGS := -march=armv8-a -O2
> >>>
> >>>For completeness, bringing 32bit compiler need to check whether the 32bit
> >>>toolchain support some options. IIRC, armv8-a support isn't enabled until
> >>>gcc 4.8, so old toolchains such gcc-4.7 will complain:
> >>> error: unrecognized argument in option ?-march=armv8-a?
> >>
> >>That's a fair point. I guess -march=armv8-a is not strictly necessary and
> >>the produced vDSO should be fine if arch/arm/vdso also compiles fine.
> >>However we would still need to pass -march=armv7-a. I'm not sure what to do
> >>between:
> >>* Checking that the compiler supports -march=armv8-a when inspecting
> >>CROSS_COMPILE_ARM32, and if it doesn't vdso32 will not be built.
> >>* Checking whether -march=armv8-a is available here, and if it is not fall
> >>back to -march=armv7-a.
> >
> >Does v8 vs v7 make any difference in the generated code? If not, we
> >could just stick to armv7-a permanently.
>
> I've just tried compiling with -march=armv7-a, and in fact it doesn't
> compile at all. It turns out vgettimeofday.c uses smp_rmb(), which expands
> to dmb ishld on arm64, and ishld doesn't exist in ARMv7. We could possibly
> work around that, but I think requiring GCC 4.8 is reasonable.
Since vgettimeofday.c is meant to be compiled for AArch32, it wouldn't
look too bad to define its own barriers rather than relying on the
AArch64 ones. So you could define v7_smp_rmb/v7_smp_wmb and use them in
this file. Alternatively, replace smp_rmb() with smp_mb() in this file
but with a big comment about ARMv7 compilation requirement and "ishld"
not being available.
--
Catalin
^ permalink raw reply
* [PATCH 0/2] ARM: davinvi: da850 add ohci DT nodes
From: Axel Haslam @ 2016-11-21 18:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4095b7bc-a6dd-144b-4a65-40d86c710087@lechnology.com>
On Mon, Nov 21, 2016 at 6:45 PM, David Lechner <david@lechnology.com> wrote:
> On 11/21/2016 11:29 AM, Axel Haslam wrote:
>>
>> On Mon, Nov 21, 2016 at 6:04 PM, David Lechner <david@lechnology.com>
>> wrote:
>>>
>>> On 11/21/2016 10:59 AM, Axel Haslam wrote:
>>>>
>>>>
>>>> This adds the DT node for the ohci controller and
>>>> enables it for the omapl138-lckd platform.
>>>>
>>>> DEPENDENCIES:
>>>>
>>>> 1. [PATCH v6 0/5] USB: ohci-da8xx: Add device tree support
>>>> https://lkml.org/lkml/2016/11/21/558
>>>>
>>>> 2. [PATCH v3 0/2] regulator: handling of error conditions for usb
>>>> drivers
>>>> https://lkml.org/lkml/2016/11/4/465
>>>>
>>>> Axel Haslam (2):
>>>> ARM: dts: da850: Add usb device node
>>>> ARM: dts: da850-lcdk: Enable ohci for omapl138 lcdk
>>>>
>>>> arch/arm/boot/dts/da850-lcdk.dts | 8 ++++++++
>>>> arch/arm/boot/dts/da850.dtsi | 8 ++++++++
>>>> 2 files changed, 16 insertions(+)
>>>>
>>>
>>> It does not look like you rebased these patches. Sekhar pushed the musb
>>> counterpart to v4.10/dt yesterday, which will cause conflicts with this
>>> series.
>>>
>>>
>>> https://git.kernel.org/cgit/linux/kernel/git/nsekhar/linux-davinci.git/log/?h=v4.10/dt
>>
>>
>> Hi David,
>>
>> i verified that they apply to the current linux-davinci/master.
>> Anyways, i can rebase
>> and resend once the dependencies are met, and we are ready to merge it.
>>
>> Regards
>> Axel.
>>
>
> OK. I think the first patch is fine, but you will have two &usb_phy {
> status = "okay"; }; in da850-lcdk.dts now even if applies cleanly. ;-)
mmm, right! ill remove it when i resend.
^ permalink raw reply
* [PATCH v2 1/4] spi: spi-fsl-dspi: Fix SPI transfer issue when using multiple SPI_IOC_MESSAGE
From: maitysanchayan at gmail.com @ 2016-11-21 19:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161121191847.vg32cwople4qmini@sirena.org.uk>
Hello Mark,
On 16-11-21 19:18:47, Mark Brown wrote:
> On Mon, Nov 21, 2016 at 11:24:01AM +0530, Sanchayan Maity wrote:
> > Current DMA implementation had a bug where the DMA transfer would
> > exit the loop in dspi_transfer_one_message after the completion of
> > a single transfer. This results in a multi message transfer submitted
> > with SPI_IOC_MESSAGE to terminate incorrectly without an error.
>
> Please don't resend already applied patches. If there are any changes
> needed please send incremental changes on top of what's already applied.
This is not a resend of an applied patch. The whole series applies on
top of your topic/fsl-dspi branch and has fixes for the SPI DMA as
incremental changes
- Sanchaya.
^ permalink raw reply
* [PATCH v2 1/4] spi: spi-fsl-dspi: Fix SPI transfer issue when using multiple SPI_IOC_MESSAGE
From: Mark Brown @ 2016-11-21 19:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <bbdbc8df434dd2af74eb351b799a2812a1c1967e.1479706671.git.maitysanchayan@gmail.com>
On Mon, Nov 21, 2016 at 11:24:01AM +0530, Sanchayan Maity wrote:
> Current DMA implementation had a bug where the DMA transfer would
> exit the loop in dspi_transfer_one_message after the completion of
> a single transfer. This results in a multi message transfer submitted
> with SPI_IOC_MESSAGE to terminate incorrectly without an error.
Please don't resend already applied patches. If there are any changes
needed please send incremental changes on top of what's already applied.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 455 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161121/59a1e93d/attachment.sig>
^ permalink raw reply
* [PATCH v2 1/4] spi: spi-fsl-dspi: Fix SPI transfer issue when using multiple SPI_IOC_MESSAGE
From: maitysanchayan at gmail.com @ 2016-11-21 19:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161121191430.GA24521@Sanchayan-Arch.localdomain>
Hello Mark,
On 16-11-22 00:44:30, maitysanchayan at gmail.com wrote:
> Hello Mark,
>
> On 16-11-21 19:18:47, Mark Brown wrote:
> > On Mon, Nov 21, 2016 at 11:24:01AM +0530, Sanchayan Maity wrote:
> > > Current DMA implementation had a bug where the DMA transfer would
> > > exit the loop in dspi_transfer_one_message after the completion of
> > > a single transfer. This results in a multi message transfer submitted
> > > with SPI_IOC_MESSAGE to terminate incorrectly without an error.
> >
> > Please don't resend already applied patches. If there are any changes
> > needed please send incremental changes on top of what's already applied.
>
> This is not a resend of an applied patch. The whole series applies on
> top of your topic/fsl-dspi branch and has fixes for the SPI DMA as
> incremental changes
>
Sorry. I take that back. I now see you applied the patch and I got the applied
mail after I replied.
- Sanchayan.
^ permalink raw reply
* Applied "spi: spi-fsl-dspi: Fix SPI transfer issue when using multiple SPI_IOC_MESSAGE" to the spi tree
From: Mark Brown @ 2016-11-21 19:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <bbdbc8df434dd2af74eb351b799a2812a1c1967e.1479384571.git.maitysanchayan@gmail.com>
The patch
spi: spi-fsl-dspi: Fix SPI transfer issue when using multiple SPI_IOC_MESSAGE
has been applied to the spi tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
>From 9811430465fccae17862213d07eba017c149eb9c Mon Sep 17 00:00:00 2001
From: Sanchayan Maity <maitysanchayan@gmail.com>
Date: Thu, 17 Nov 2016 17:46:48 +0530
Subject: [PATCH] spi: spi-fsl-dspi: Fix SPI transfer issue when using multiple
SPI_IOC_MESSAGE
Current DMA implementation had a bug where the DMA transfer would
exit the loop in dspi_transfer_one_message after the completion of
a single transfer. This results in a multi message transfer submitted
with SPI_IOC_MESSAGE to terminate incorrectly without an error.
Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
Reviewed-by: Stefan Agner <stefan@agner.ch>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
drivers/spi/spi-fsl-dspi.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index bc64700b514d..b1ee1f521ba0 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -714,7 +714,7 @@ static int dspi_transfer_one_message(struct spi_master *master,
SPI_RSER_TFFFE | SPI_RSER_TFFFD |
SPI_RSER_RFDFE | SPI_RSER_RFDFD);
status = dspi_dma_xfer(dspi);
- goto out;
+ break;
default:
dev_err(&dspi->pdev->dev, "unsupported trans_mode %u\n",
trans_mode);
@@ -722,9 +722,13 @@ static int dspi_transfer_one_message(struct spi_master *master,
goto out;
}
- if (wait_event_interruptible(dspi->waitq, dspi->waitflags))
- dev_err(&dspi->pdev->dev, "wait transfer complete fail!\n");
- dspi->waitflags = 0;
+ if (trans_mode != DSPI_DMA_MODE) {
+ if (wait_event_interruptible(dspi->waitq,
+ dspi->waitflags))
+ dev_err(&dspi->pdev->dev,
+ "wait transfer complete fail!\n");
+ dspi->waitflags = 0;
+ }
if (transfer->delay_usecs)
udelay(transfer->delay_usecs);
--
2.10.2
^ permalink raw reply related
* [PATCH 0/2] arm64: dts: NS2: Add sdio1, PCI phys
From: Florian Fainelli @ 2016-11-21 19:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479425103-2119-1-git-send-email-jon.mason@broadcom.com>
On 11/17/2016 03:25 PM, Jon Mason wrote:
> The second SDIO seems to have been forgotten to be enabled in the
> Northstar2 SVK dts. Also, the PCI PHY entries are missing from the PCI
> bus device tree entries.
Series applied, thanks Jon
--
Florian
^ permalink raw reply
* [PATCH] PCI: Add information about describing PCI in ACPI
From: Bjorn Helgaas @ 2016-11-21 20:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <EE11001F9E5DDD47B7634E2F8A612F2E1F929566@lhreml507-mbx>
On Mon, Nov 21, 2016 at 05:23:11PM +0000, Gabriele Paoloni wrote:
> Hi Bjorn
>
> > -----Original Message-----
> > From: linux-pci-owner at vger.kernel.org [mailto:linux-pci-
> > owner at vger.kernel.org] On Behalf Of Bjorn Helgaas
> > Sent: 21 November 2016 16:47
> > To: Gabriele Paoloni
> > Cc: Bjorn Helgaas; linux-pci at vger.kernel.org; linux-
> > acpi at vger.kernel.org; linux-kernel at vger.kernel.org; linux-arm-
> > kernel at lists.infradead.org; linaro-acpi at lists.linaro.org
> > Subject: Re: [PATCH] PCI: Add information about describing PCI in ACPI
> >
> > On Mon, Nov 21, 2016 at 08:52:52AM +0000, Gabriele Paoloni wrote:
> > > Hi Bjorn
> > >
> > > > -----Original Message-----
> > > > From: Bjorn Helgaas [mailto:helgaas at kernel.org]
> > > > Sent: 18 November 2016 17:54
> > > > To: Gabriele Paoloni
> > > > Cc: Bjorn Helgaas; linux-pci at vger.kernel.org; linux-
> > > > acpi at vger.kernel.org; linux-kernel at vger.kernel.org; linux-arm-
> > > > kernel at lists.infradead.org; linaro-acpi at lists.linaro.org
> > > > Subject: Re: [PATCH] PCI: Add information about describing PCI in
> > ACPI
> > > >
> > > > On Fri, Nov 18, 2016 at 05:17:34PM +0000, Gabriele Paoloni wrote:
> > > > > > -----Original Message-----
> > > > > > From: linux-kernel-owner at vger.kernel.org [mailto:linux-kernel-
> > > > > > owner at vger.kernel.org] On Behalf Of Bjorn Helgaas
> > > > > > Sent: 17 November 2016 18:00
> > > >
> > > > > > +Static tables like MCFG, HPET, ECDT, etc., are *not*
> > mechanisms
> > > > for
> > > > > > +reserving address space! The static tables are for things the
> > OS
> > > > > > +needs to know early in boot, before it can parse the ACPI
> > > > namespace.
> > > > > > +If a new table is defined, an old OS needs to operate
> > correctly
> > > > even
> > > > > > +though it ignores the table. _CRS allows that because it is
> > > > generic
> > > > > > +and understood by the old OS; a static table does not.
> > > > >
> > > > > Right so if my understanding is correct you are saying that
> > resources
> > > > > described in the MCFG table should also be declared in PNP0C02
> > > > devices
> > > > > so that the PNP driver can reserve these resources.
> > > >
> > > > Yes.
> > > >
> > > > > On the other side the PCI Root bridge driver should not reserve
> > such
> > > > > resources.
> > > > >
> > > > > Well if my understanding is correct I think we have a problem
> > here:
> > > > > http://lxr.free-electrons.com/source/drivers/pci/ecam.c#L74
> > > > >
> > > > > As you can see pci_ecam_create() will conflict with the pnp
> > driver
> > > > > as it will try to reserve the resources from the MCFG table...
> > > > >
> > > > > Maybe we need to rework pci_ecam_create() ?
> > > >
> > > > I think it's OK as it is.
> > > >
> > > > The pnp/system.c driver does try to reserve PNP0C02 resources, and
> > it
> > > > marks them as "not busy". That way they appear in /proc/iomem and
> > > > won't be allocated for anything else, but they can still be
> > requested
> > > > by drivers, e.g., pci/ecam.c, which will mark them "busy".
> > > >
> > > > This is analogous to what the PCI core does in
> > pci_claim_resource().
> > > > This is really a function of the ACPI/PNP *core*, which should
> > reserve
> > > > all _CRS resources for all devices (not just PNP0C02 devices). But
> > > > it's done by pnp/system.c, and only for PNP0C02, because there's a
> > > > bunch of historical baggage there.
> > > >
> > > > You'll also notice that in this case, things are out of order:
> > > > logically the pnp/system.c reservation should happen first, but in
> > > > fact the pci/ecam.c request happens *before* the pnp/system.c one.
> > > > That means the pnp/system.c one might fail and complain "[mem ...]
> > > > could not be reserved".
> > >
> > > Correct me if I am wrong...
> > >
> > > So currently we are relying on the fact that pci_ecam_create() is
> > called
> > > before the pnp driver.
> > > If the pnp driver came first we would end up in pci_ecam_create()
> > failing
> > > here:
> > > http://lxr.free-electrons.com/source/drivers/pci/ecam.c#L76
> > >
> > > I am not sure but it seems to me like a bit weak condition to rely
> > on...
> > > what about removing the error condition in pci_ecam_create() and
> > logging
> > > just a dev_info()?
> >
> > Huh. I'm confused. I *thought* it would be safe to reverse the
> > order, which would effectively be this:
> >
> > system_pnp_probe
> > reserve_resources_of_dev
> > reserve_range
> > request_mem_region([mem 0xb0000000-0xb1ffffff])
> > ...
> > pci_ecam_create
> > request_resource_conflict([mem 0xb0000000-0xb1ffffff])
> >
> >
> > but I experimented with the patch below on qemu, and it failed as you
> > predicted:
> >
> > ** res test **
> > requested [mem 0xa0000000-0xafffffff]
> > can't claim ECAM area [mem 0xa0000000-0xafffffff]: conflict with ECAM
> > PNP [mem 0xa0000000-0xafffffff]
> >
> > I expected the request_resource_conflict() to succeed since it's
> > completely contained in the "ECAM PNP" region. But I guess I don't
> > understand kernel/resource.c well enough.
>
> I think it fails because effectively the PNP driver is populating the
> iomem_resource resource tree and therefore pci_ecam_create() finds that
> it cannot add the cfg resource to the same hierarchy as it is already
> there...
Right. I'm just surprised because the PNP reservation is marked
"not busy", and a driver (e.g., ECAM) should still be able to request
the resource.
> > I'm not sure we need to fix anything yet, since we currently do the
> > ecam.c request before the system.c one, and any change there would be
> > a long ways off. If/when that *does* change, I think the correct fix
> > would be to change ecam.c so its request succeeds (by changing the way
> > it does the request, fixing kernel/resource.c, or whatever) rather
> > than to reduce the log level and ignore the failure.
>
> Well in my mind I didn't want just to make the error disappear...
> If all the resources should be reserved by the PNP driver then ideally
> we could take away request_resource_conflict() from pci_ecam_create(),
> but this would make buggy some systems with an already shipped BIOS
> that relied on pci_ecam_create() reservation rather than PNP reservation.
I don't want remove the request from ecam.c. Ideally, there should be
TWO lines in /proc/iomem: one from system.c for "pnp 00:01" or
whatever it is, and a second from ecam.c. The first is the generic
one saying "this region is consumed by a piece of hardware, so don't
put anything else here." The second is the driver-specific one saying
"PCI ECAM owns this region, nobody else can use it."
This is the same way we handle PCI BAR resources. Here are two
examples from my laptop. The first (00:08.0) only has one line:
it has a BAR that consumes address space, but I don't have a driver
for it loaded. The second (00:16.0) does have a driver loaded, so it
has a second line showing that the driver owns the space:
f124a000-f124afff : 0000:00:08.0 # from PCI core
f124d000-f124dfff : 0000:00:16.0 # from PCI core
f124d000-f124dfff : mei_me # from mei_me driver
> Just removing the error condition and converting dev_err() into
> dev_info() seems to me like accommodating already shipped BIOS images
> and flagging a reservation that is already done by somebody else
> without compromising the functionality of the PCI Root bridge driver
> (so far the only reason why I can see the error condition there is
> to catch a buggy MCFG with overlapping addresses; so if this is the
> case maybe we need to have a different diagnostic check to make sure
> that the MCFG table is alright)
Ideally I think we should end up with this:
a0000000-afffffff : pnp 00:01
a0000000-afffffff : PCI ECAM
Realistically right now we'll probably end up with only the "PCI ECAM"
line in /proc/iomem and a warning from system.c about not being able
to reserve the space.
If we ever change things to do the generic PNP reservation first, then
we should fix things so ecam.c can claim the space without an error.
^ permalink raw reply
* [kvm-unit-tests PATCH v10 0/3] ARM PMU tests
From: Wei Huang @ 2016-11-21 20:24 UTC (permalink / raw)
To: linux-arm-kernel
Changes from v9:
* Move PMCCNTR related configuration from pmu_init() to sub-tests
* Change the name of loop test function to precise_cycles_loop()
* Print out error detail for each test case in check_cpi()
* Fix cpi convertion from argv
* Change the loop calculation in measure_instrs() after cpi is fixed
Note:
1) Current KVM code has bugs in handling PMCCFILTR write. A fix (see
below) is required for this unit testing code to work correctly under
KVM mode.
https://lists.cs.columbia.edu/pipermail/kvmarm/2016-November/022134.html.
Thanks,
-Wei
Christopher Covington (3):
arm: Add PMU test
arm: pmu: Check cycle count increases
arm: pmu: Add CPI checking
arm/Makefile.common | 3 +-
arm/pmu.c | 347 ++++++++++++++++++++++++++++++++++++++++++++++++++++
arm/unittests.cfg | 19 +++
3 files changed, 368 insertions(+), 1 deletion(-)
create mode 100644 arm/pmu.c
--
1.8.3.1
^ permalink raw reply
* [kvm-unit-tests PATCH v10 1/3] arm: Add PMU test
From: Wei Huang @ 2016-11-21 20:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479759895-10042-1-git-send-email-wei@redhat.com>
From: Christopher Covington <cov@codeaurora.org>
Beginning with a simple sanity check of the control register, add
a unit test for the ARM Performance Monitors Unit (PMU).
Signed-off-by: Christopher Covington <cov@codeaurora.org>
Signed-off-by: Wei Huang <wei@redhat.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
---
arm/Makefile.common | 3 ++-
arm/pmu.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++
arm/unittests.cfg | 5 ++++
3 files changed, 81 insertions(+), 1 deletion(-)
create mode 100644 arm/pmu.c
diff --git a/arm/Makefile.common b/arm/Makefile.common
index f37b5c2..5da2fdd 100644
--- a/arm/Makefile.common
+++ b/arm/Makefile.common
@@ -12,7 +12,8 @@ endif
tests-common = \
$(TEST_DIR)/selftest.flat \
$(TEST_DIR)/spinlock-test.flat \
- $(TEST_DIR)/pci-test.flat
+ $(TEST_DIR)/pci-test.flat \
+ $(TEST_DIR)/pmu.flat
all: test_cases
diff --git a/arm/pmu.c b/arm/pmu.c
new file mode 100644
index 0000000..9d9c53b
--- /dev/null
+++ b/arm/pmu.c
@@ -0,0 +1,74 @@
+/*
+ * Test the ARM Performance Monitors Unit (PMU).
+ *
+ * Copyright (c) 2015-2016, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License version 2.1 and
+ * only version 2.1 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License
+ * for more details.
+ */
+#include "libcflat.h"
+#include "asm/barrier.h"
+
+#define PMU_PMCR_N_SHIFT 11
+#define PMU_PMCR_N_MASK 0x1f
+#define PMU_PMCR_ID_SHIFT 16
+#define PMU_PMCR_ID_MASK 0xff
+#define PMU_PMCR_IMP_SHIFT 24
+#define PMU_PMCR_IMP_MASK 0xff
+
+#if defined(__arm__)
+static inline uint32_t pmcr_read(void)
+{
+ uint32_t ret;
+
+ asm volatile("mrc p15, 0, %0, c9, c12, 0" : "=r" (ret));
+ return ret;
+}
+#elif defined(__aarch64__)
+static inline uint32_t pmcr_read(void)
+{
+ uint32_t ret;
+
+ asm volatile("mrs %0, pmcr_el0" : "=r" (ret));
+ return ret;
+}
+#endif
+
+/*
+ * As a simple sanity check on the PMCR_EL0, ensure the implementer field isn't
+ * null. Also print out a couple other interesting fields for diagnostic
+ * purposes. For example, as of fall 2016, QEMU TCG mode doesn't implement
+ * event counters and therefore reports zero event counters, but hopefully
+ * support for at least the instructions event will be added in the future and
+ * the reported number of event counters will become nonzero.
+ */
+static bool check_pmcr(void)
+{
+ uint32_t pmcr;
+
+ pmcr = pmcr_read();
+
+ printf("PMU implementer: %c\n",
+ (pmcr >> PMU_PMCR_IMP_SHIFT) & PMU_PMCR_IMP_MASK);
+ printf("Identification code: 0x%x\n",
+ (pmcr >> PMU_PMCR_ID_SHIFT) & PMU_PMCR_ID_MASK);
+ printf("Event counters: %d\n",
+ (pmcr >> PMU_PMCR_N_SHIFT) & PMU_PMCR_N_MASK);
+
+ return ((pmcr >> PMU_PMCR_IMP_SHIFT) & PMU_PMCR_IMP_MASK) != 0;
+}
+
+int main(void)
+{
+ report_prefix_push("pmu");
+
+ report("Control register", check_pmcr());
+
+ return report_summary();
+}
diff --git a/arm/unittests.cfg b/arm/unittests.cfg
index ae32a42..816f494 100644
--- a/arm/unittests.cfg
+++ b/arm/unittests.cfg
@@ -58,3 +58,8 @@ groups = selftest
[pci-test]
file = pci-test.flat
groups = pci
+
+# Test PMU support
+[pmu]
+file = pmu.flat
+groups = pmu
--
1.8.3.1
^ permalink raw reply related
* [kvm-unit-tests PATCH v10 2/3] arm: pmu: Check cycle count increases
From: Wei Huang @ 2016-11-21 20:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479759895-10042-1-git-send-email-wei@redhat.com>
From: Christopher Covington <cov@codeaurora.org>
Ensure that reads of the PMCCNTR_EL0 are monotonically increasing,
even for the smallest delta of two subsequent reads.
Signed-off-by: Christopher Covington <cov@codeaurora.org>
Signed-off-by: Wei Huang <wei@redhat.com>
---
arm/pmu.c | 156 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 156 insertions(+)
diff --git a/arm/pmu.c b/arm/pmu.c
index 9d9c53b..176b070 100644
--- a/arm/pmu.c
+++ b/arm/pmu.c
@@ -15,6 +15,9 @@
#include "libcflat.h"
#include "asm/barrier.h"
+#define PMU_PMCR_E (1 << 0)
+#define PMU_PMCR_C (1 << 2)
+#define PMU_PMCR_LC (1 << 6)
#define PMU_PMCR_N_SHIFT 11
#define PMU_PMCR_N_MASK 0x1f
#define PMU_PMCR_ID_SHIFT 16
@@ -22,6 +25,14 @@
#define PMU_PMCR_IMP_SHIFT 24
#define PMU_PMCR_IMP_MASK 0xff
+#define ID_DFR0_PERFMON_SHIFT 24
+#define ID_DFR0_PERFMON_MASK 0xf
+
+#define PMU_CYCLE_IDX 31
+
+#define NR_SAMPLES 10
+
+static unsigned int pmu_version;
#if defined(__arm__)
static inline uint32_t pmcr_read(void)
{
@@ -30,6 +41,69 @@ static inline uint32_t pmcr_read(void)
asm volatile("mrc p15, 0, %0, c9, c12, 0" : "=r" (ret));
return ret;
}
+
+static inline void pmcr_write(uint32_t value)
+{
+ asm volatile("mcr p15, 0, %0, c9, c12, 0" : : "r" (value));
+ isb();
+}
+
+static inline void pmselr_write(uint32_t value)
+{
+ asm volatile("mcr p15, 0, %0, c9, c12, 5" : : "r" (value));
+ isb();
+}
+
+static inline void pmxevtyper_write(uint32_t value)
+{
+ asm volatile("mcr p15, 0, %0, c9, c13, 1" : : "r" (value));
+}
+
+static inline uint64_t pmccntr_read(void)
+{
+ uint32_t lo, hi = 0;
+
+ if (pmu_version == 0x3)
+ asm volatile("mrrc p15, 0, %0, %1, c9" : "=r" (lo), "=r" (hi));
+ else
+ asm volatile("mrc p15, 0, %0, c9, c13, 0" : "=r" (lo));
+
+ return ((uint64_t)hi << 32) | lo;
+}
+
+static inline void pmccntr_write(uint64_t value)
+{
+ uint32_t lo, hi;
+
+ lo = value & 0xffffffff;
+ hi = (value >> 32) & 0xffffffff;
+
+ if (pmu_version == 0x3)
+ asm volatile("mcrr p15, 0, %0, %1, c9" : : "r" (lo), "r" (hi));
+ else
+ asm volatile("mcr p15, 0, %0, c9, c13, 0" : : "r" (lo));
+}
+
+static inline void pmcntenset_write(uint32_t value)
+{
+ asm volatile("mcr p15, 0, %0, c9, c12, 1" : : "r" (value));
+}
+
+/* PMCCFILTR is an obsolete name for PMXEVTYPER31 in ARMv7 */
+static inline void pmccfiltr_write(uint32_t value)
+{
+ pmselr_write(PMU_CYCLE_IDX);
+ pmxevtyper_write(value);
+ isb();
+}
+
+static inline uint32_t id_dfr0_read(void)
+{
+ uint32_t val;
+
+ asm volatile("mrc p15, 0, %0, c0, c1, 2" : "=r" (val));
+ return val;
+}
#elif defined(__aarch64__)
static inline uint32_t pmcr_read(void)
{
@@ -38,6 +112,44 @@ static inline uint32_t pmcr_read(void)
asm volatile("mrs %0, pmcr_el0" : "=r" (ret));
return ret;
}
+
+static inline void pmcr_write(uint32_t value)
+{
+ asm volatile("msr pmcr_el0, %0" : : "r" (value));
+ isb();
+}
+
+static inline uint64_t pmccntr_read(void)
+{
+ uint64_t cycles;
+
+ asm volatile("mrs %0, pmccntr_el0" : "=r" (cycles));
+ return cycles;
+}
+
+static inline void pmccntr_write(uint64_t value)
+{
+ asm volatile("msr pmccntr_el0, %0" : : "r" (value));
+}
+
+static inline void pmcntenset_write(uint32_t value)
+{
+ asm volatile("msr pmcntenset_el0, %0" : : "r" (value));
+}
+
+static inline void pmccfiltr_write(uint32_t value)
+{
+ asm volatile("msr pmccfiltr_el0, %0" : : "r" (value));
+ isb();
+}
+
+static inline uint32_t id_dfr0_read(void)
+{
+ uint32_t id;
+
+ asm volatile("mrs %0, id_dfr0_el1" : "=r" (id));
+ return id;
+}
#endif
/*
@@ -64,11 +176,55 @@ static bool check_pmcr(void)
return ((pmcr >> PMU_PMCR_IMP_SHIFT) & PMU_PMCR_IMP_MASK) != 0;
}
+/*
+ * Ensure that the cycle counter progresses between back-to-back reads.
+ */
+static bool check_cycles_increase(void)
+{
+ bool success = true;
+
+ /* init before event access, this test only cares about cycle count */
+ pmcntenset_write(1 << PMU_CYCLE_IDX);
+ pmccfiltr_write(0); /* count cycles in EL0, EL1, but not EL2 */
+ pmccntr_write(0);
+
+ pmcr_write(pmcr_read() | PMU_PMCR_LC | PMU_PMCR_C | PMU_PMCR_E);
+
+ for (int i = 0; i < NR_SAMPLES; i++) {
+ uint64_t a, b;
+
+ a = pmccntr_read();
+ b = pmccntr_read();
+
+ if (a >= b) {
+ printf("Read %"PRId64" then %"PRId64".\n", a, b);
+ success = false;
+ break;
+ }
+ }
+
+ pmcr_write(pmcr_read() & ~PMU_PMCR_E);
+
+ return success;
+}
+
+void pmu_init(void)
+{
+ uint32_t dfr0;
+
+ /* probe pmu version */
+ dfr0 = id_dfr0_read();
+ pmu_version = (dfr0 >> ID_DFR0_PERFMON_SHIFT) & ID_DFR0_PERFMON_MASK;
+ printf("PMU version: %d\n", pmu_version);
+}
+
int main(void)
{
report_prefix_push("pmu");
+ pmu_init();
report("Control register", check_pmcr());
+ report("Monotonically increasing cycle count", check_cycles_increase());
return report_summary();
}
--
1.8.3.1
^ permalink raw reply related
* [kvm-unit-tests PATCH v10 3/3] arm: pmu: Add CPI checking
From: Wei Huang @ 2016-11-21 20:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479759895-10042-1-git-send-email-wei@redhat.com>
From: Christopher Covington <cov@codeaurora.org>
Calculate the numbers of cycles per instruction (CPI) implied by ARM
PMU cycle counter values. The code includes a strict checking facility
intended for the -icount option in TCG mode in the configuration file.
Signed-off-by: Christopher Covington <cov@codeaurora.org>
Signed-off-by: Wei Huang <wei@redhat.com>
---
arm/pmu.c | 119 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
arm/unittests.cfg | 14 +++++++
2 files changed, 132 insertions(+), 1 deletion(-)
diff --git a/arm/pmu.c b/arm/pmu.c
index 176b070..129ef1e 100644
--- a/arm/pmu.c
+++ b/arm/pmu.c
@@ -104,6 +104,25 @@ static inline uint32_t id_dfr0_read(void)
asm volatile("mrc p15, 0, %0, c0, c1, 2" : "=r" (val));
return val;
}
+
+/*
+ * Extra instructions inserted by the compiler would be difficult to compensate
+ * for, so hand assemble everything between, and including, the PMCR accesses
+ * to start and stop counting. Total cycles = isb + mcr + 2*loop = 2 + 2*loop.
+ */
+static inline void precise_cycles_loop(int loop, uint32_t pmcr)
+{
+ asm volatile(
+ " mcr p15, 0, %[pmcr], c9, c12, 0\n"
+ " isb\n"
+ "1: subs %[loop], %[loop], #1\n"
+ " bgt 1b\n"
+ " mcr p15, 0, %[z], c9, c12, 0\n"
+ " isb\n"
+ : [loop] "+r" (loop)
+ : [pmcr] "r" (pmcr), [z] "r" (0)
+ : "cc");
+}
#elif defined(__aarch64__)
static inline uint32_t pmcr_read(void)
{
@@ -150,6 +169,25 @@ static inline uint32_t id_dfr0_read(void)
asm volatile("mrs %0, id_dfr0_el1" : "=r" (id));
return id;
}
+
+/*
+ * Extra instructions inserted by the compiler would be difficult to compensate
+ * for, so hand assemble everything between, and including, the PMCR accesses
+ * to start and stop counting. Total cycles = isb + msr + 2*loop = 2 + 2*loop.
+ */
+static inline void precise_cycles_loop(int loop, uint32_t pmcr)
+{
+ asm volatile(
+ " msr pmcr_el0, %[pmcr]\n"
+ " isb\n"
+ "1: subs %[loop], %[loop], #1\n"
+ " b.gt 1b\n"
+ " msr pmcr_el0, xzr\n"
+ " isb\n"
+ : [loop] "+r" (loop)
+ : [pmcr] "r" (pmcr)
+ : "cc");
+}
#endif
/*
@@ -208,6 +246,79 @@ static bool check_cycles_increase(void)
return success;
}
+/*
+ * Execute a known number of guest instructions. Only odd instruction counts
+ * greater than or equal to 3 are supported by the in-line assembly code. The
+ * control register (PMCR_EL0) is initialized with the provided value (allowing
+ * for example for the cycle counter or event counters to be reset). At the end
+ * of the exact instruction loop, zero is written to PMCR_EL0 to disable
+ * counting, allowing the cycle counter or event counters to be read at the
+ * leisure of the calling code.
+ */
+static void measure_instrs(int num, uint32_t pmcr)
+{
+ int loop = (num - 2) / 2;
+
+ assert(num >= 4 && ((num - 2) % 2 == 0));
+ precise_cycles_loop(loop, pmcr);
+}
+
+/*
+ * Measure cycle counts for various known instruction counts. Ensure that the
+ * cycle counter progresses (similar to check_cycles_increase() but with more
+ * instructions and using reset and stop controls). If supplied a positive,
+ * nonzero CPI parameter, also strictly check that every measurement matches
+ * it. Strict CPI checking is used to test -icount mode.
+ */
+static bool check_cpi(int cpi)
+{
+ uint32_t pmcr = pmcr_read() | PMU_PMCR_LC | PMU_PMCR_C | PMU_PMCR_E;
+
+ /* init before event access, this test only cares about cycle count */
+ pmcntenset_write(1 << PMU_CYCLE_IDX);
+ pmccfiltr_write(0); /* count cycles in EL0, EL1, but not EL2 */
+
+ if (cpi > 0)
+ printf("Checking for CPI=%d.\n", cpi);
+ printf("instrs : cycles0 cycles1 ...\n");
+
+ for (unsigned int i = 4; i < 300; i += 32) {
+ uint64_t avg, sum = 0;
+
+ printf("%d :", i);
+ for (int j = 0; j < NR_SAMPLES; j++) {
+ uint64_t cycles;
+
+ pmccntr_write(0);
+ measure_instrs(i, pmcr);
+ cycles = pmccntr_read();
+ printf(" %"PRId64"", cycles);
+
+ if (!cycles) {
+ printf("\ncycles not incrementing!\n");
+ return false;
+ } else if (cpi > 0 && cycles != i * cpi) {
+ printf("\nunexpected cycle count received!\n");
+ return false;
+ } else if ((cycles >> 32) != 0) {
+ /* The cycles taken by the loop above should
+ * fit in 32 bits easily. We check the upper
+ * 32 bits of the cycle counter to make sure
+ * there is no supprise. */
+ printf("\ncycle count bigger than 32bit!\n");
+ return false;
+ }
+
+ sum += cycles;
+ }
+ avg = sum / NR_SAMPLES;
+ printf(" sum=%"PRId64" avg=%"PRId64" avg_ipc=%"PRId64" "
+ "avg_cpi=%"PRId64"\n", sum, avg, i / avg, avg / i);
+ }
+
+ return true;
+}
+
void pmu_init(void)
{
uint32_t dfr0;
@@ -218,13 +329,19 @@ void pmu_init(void)
printf("PMU version: %d\n", pmu_version);
}
-int main(void)
+int main(int argc, char *argv[])
{
+ int cpi = 0;
+
+ if (argc > 1)
+ cpi = atol(argv[1]);
+
report_prefix_push("pmu");
pmu_init();
report("Control register", check_pmcr());
report("Monotonically increasing cycle count", check_cycles_increase());
+ report("Cycle/instruction ratio", check_cpi(cpi));
return report_summary();
}
diff --git a/arm/unittests.cfg b/arm/unittests.cfg
index 816f494..044d97c 100644
--- a/arm/unittests.cfg
+++ b/arm/unittests.cfg
@@ -63,3 +63,17 @@ groups = pci
[pmu]
file = pmu.flat
groups = pmu
+
+# Test PMU support (TCG) with -icount IPC=1
+[pmu-tcg-icount-1]
+file = pmu.flat
+extra_params = -icount 0 -append '1'
+groups = pmu
+accel = tcg
+
+# Test PMU support (TCG) with -icount IPC=256
+[pmu-tcg-icount-256]
+file = pmu.flat
+extra_params = -icount 8 -append '256'
+groups = pmu
+accel = tcg
--
1.8.3.1
^ permalink raw reply related
* [PATCH] ARM: lpc32xx: drop duplicate header device.h
From: Sylvain Lemieux @ 2016-11-21 20:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <31c0875cedae8fe20211b209c85b6e012fa9f6a5.1479457940.git.geliangtang@gmail.com>
On Fri, 2016-11-18 at 22:21 +0800, Geliang Tang wrote:
> Drop duplicate header device.h from phy3250.c.
>
> Signed-off-by: Geliang Tang <geliangtang@gmail.com>
> ---
> arch/arm/mach-lpc32xx/phy3250.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/arch/arm/mach-lpc32xx/phy3250.c b/arch/arm/mach-lpc32xx/phy3250.c
> index 0e4cbbe..6c52bd3 100644
> --- a/arch/arm/mach-lpc32xx/phy3250.c
> +++ b/arch/arm/mach-lpc32xx/phy3250.c
> @@ -23,7 +23,6 @@
> #include <linux/interrupt.h>
> #include <linux/irq.h>
> #include <linux/dma-mapping.h>
> -#include <linux/device.h>
> #include <linux/gpio.h>
> #include <linux/amba/bus.h>
> #include <linux/amba/clcd.h>
Reviewed-by: Sylvain Lemieux <slemieux.tyco@gmail.com>
^ permalink raw reply
* [kvm-unit-tests PATCH v10 3/3] arm: pmu: Add CPI checking
From: Christopher Covington @ 2016-11-21 21:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479759895-10042-4-git-send-email-wei@redhat.com>
Hi Wei,
On 11/21/2016 03:24 PM, Wei Huang wrote:
> From: Christopher Covington <cov@codeaurora.org>
I really appreciate your work on these patches. If for any or all of these
you have more lines added/modified than me (or using any other better
metric), please make sure to change the author to be you with
`git commit --amend --reset-author` or equivalent.
> Calculate the numbers of cycles per instruction (CPI) implied by ARM
> PMU cycle counter values. The code includes a strict checking facility
> intended for the -icount option in TCG mode in the configuration file.
>
> Signed-off-by: Christopher Covington <cov@codeaurora.org>
> Signed-off-by: Wei Huang <wei@redhat.com>
> ---
> arm/pmu.c | 119 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
> arm/unittests.cfg | 14 +++++++
> 2 files changed, 132 insertions(+), 1 deletion(-)
>
> diff --git a/arm/pmu.c b/arm/pmu.c
> index 176b070..129ef1e 100644
> --- a/arm/pmu.c
> +++ b/arm/pmu.c
> @@ -104,6 +104,25 @@ static inline uint32_t id_dfr0_read(void)
> asm volatile("mrc p15, 0, %0, c0, c1, 2" : "=r" (val));
> return val;
> }
> +
> +/*
> + * Extra instructions inserted by the compiler would be difficult to compensate
> + * for, so hand assemble everything between, and including, the PMCR accesses
> + * to start and stop counting. Total cycles = isb + mcr + 2*loop = 2 + 2*loop.
> + */
> +static inline void precise_cycles_loop(int loop, uint32_t pmcr)
Nit: I would call this precise_instrs_loop. How many cycles it takes is
IMPLEMENTATION DEFINED.
> +{
> + asm volatile(
> + " mcr p15, 0, %[pmcr], c9, c12, 0\n"
> + " isb\n"
> + "1: subs %[loop], %[loop], #1\n"
> + " bgt 1b\n"
Is there any chance we might need an isb here, to prevent the stop from happening
before or during the loop? Where ISBs are required, the Linux best practice is to
diligently comment why they are needed. Perhaps it would be a good habit to
carry over into kvm-unit-tests.
> + " mcr p15, 0, %[z], c9, c12, 0\n"
> + " isb\n"
> + : [loop] "+r" (loop)
> + : [pmcr] "r" (pmcr), [z] "r" (0)
> + : "cc");
> +}
> #elif defined(__aarch64__)
> static inline uint32_t pmcr_read(void)
> {
> @@ -150,6 +169,25 @@ static inline uint32_t id_dfr0_read(void)
> asm volatile("mrs %0, id_dfr0_el1" : "=r" (id));
> return id;
> }
> +
> +/*
> + * Extra instructions inserted by the compiler would be difficult to compensate
> + * for, so hand assemble everything between, and including, the PMCR accesses
> + * to start and stop counting. Total cycles = isb + msr + 2*loop = 2 + 2*loop.
> + */
> +static inline void precise_cycles_loop(int loop, uint32_t pmcr)
> +{
> + asm volatile(
> + " msr pmcr_el0, %[pmcr]\n"
> + " isb\n"
> + "1: subs %[loop], %[loop], #1\n"
> + " b.gt 1b\n"
> + " msr pmcr_el0, xzr\n"
> + " isb\n"
> + : [loop] "+r" (loop)
> + : [pmcr] "r" (pmcr)
> + : "cc");
> +}
> #endif
>
> /*
> @@ -208,6 +246,79 @@ static bool check_cycles_increase(void)
> return success;
> }
>
> +/*
> + * Execute a known number of guest instructions. Only odd instruction counts
> + * greater than or equal to 3 are supported by the in-line assembly code. The
Nit: needs updating as well (or removal if you prefer)
> + * control register (PMCR_EL0) is initialized with the provided value (allowing
> + * for example for the cycle counter or event counters to be reset). At the end
> + * of the exact instruction loop, zero is written to PMCR_EL0 to disable
> + * counting, allowing the cycle counter or event counters to be read at the
> + * leisure of the calling code.
> + */
> +static void measure_instrs(int num, uint32_t pmcr)
> +{
> + int loop = (num - 2) / 2;
> +
> + assert(num >= 4 && ((num - 2) % 2 == 0));
> + precise_cycles_loop(loop, pmcr);
> +}
> +
> +/*
> + * Measure cycle counts for various known instruction counts. Ensure that the
> + * cycle counter progresses (similar to check_cycles_increase() but with more
> + * instructions and using reset and stop controls). If supplied a positive,
> + * nonzero CPI parameter, also strictly check that every measurement matches
> + * it. Strict CPI checking is used to test -icount mode.
> + */
> +static bool check_cpi(int cpi)
> +{
> + uint32_t pmcr = pmcr_read() | PMU_PMCR_LC | PMU_PMCR_C | PMU_PMCR_E;
> +
> + /* init before event access, this test only cares about cycle count */
> + pmcntenset_write(1 << PMU_CYCLE_IDX);
> + pmccfiltr_write(0); /* count cycles in EL0, EL1, but not EL2 */
> +
> + if (cpi > 0)
> + printf("Checking for CPI=%d.\n", cpi);
> + printf("instrs : cycles0 cycles1 ...\n");
> +
> + for (unsigned int i = 4; i < 300; i += 32) {
> + uint64_t avg, sum = 0;
> +
> + printf("%d :", i);
> + for (int j = 0; j < NR_SAMPLES; j++) {
> + uint64_t cycles;
> +
> + pmccntr_write(0);
> + measure_instrs(i, pmcr);
> + cycles = pmccntr_read();
> + printf(" %"PRId64"", cycles);
> +
> + if (!cycles) {
> + printf("\ncycles not incrementing!\n");
> + return false;
> + } else if (cpi > 0 && cycles != i * cpi) {
> + printf("\nunexpected cycle count received!\n");
> + return false;
> + } else if ((cycles >> 32) != 0) {
> + /* The cycles taken by the loop above should
> + * fit in 32 bits easily. We check the upper
> + * 32 bits of the cycle counter to make sure
> + * there is no supprise. */
> + printf("\ncycle count bigger than 32bit!\n");
> + return false;
> + }
> +
> + sum += cycles;
> + }
> + avg = sum / NR_SAMPLES;
> + printf(" sum=%"PRId64" avg=%"PRId64" avg_ipc=%"PRId64" "
> + "avg_cpi=%"PRId64"\n", sum, avg, i / avg, avg / i);
> + }
> +
> + return true;
> +}
> +
> void pmu_init(void)
> {
> uint32_t dfr0;
> @@ -218,13 +329,19 @@ void pmu_init(void)
> printf("PMU version: %d\n", pmu_version);
> }
This is clearly the right feature register to check. Sorry to have
gotten excited about an inferior method.
Cov
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code
Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply
* [PATCH 6/10] clk: sunxi-ng: Add A10s CCU driver
From: Maxime Ripard @ 2016-11-21 21:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAGb2v64cft9nWh13c+8jG8qhrD_RnVFc+s8btMTHbdDS155eQA@mail.gmail.com>
On Sun, Nov 13, 2016 at 05:34:44PM +0800, Chen-Yu Tsai wrote:
> > +static struct ccu_nkmp pll_ve_clk = {
> > + .enable = BIT(31),
> > + .n = _SUNXI_CCU_MULT_OFFSET(8, 5, 0),
> > + .k = _SUNXI_CCU_MULT(4, 2),
> > + .m = _SUNXI_CCU_DIV(0, 2),
> > + .p = _SUNXI_CCU_DIV(16, 2),
>
> Any chance we'll support the bypass switch on this one?
I'm not sure this will be useful to have it running at 24MHz.
> > +static SUNXI_CCU_M(pll_ddr_clk, "pll-ddr", "pll-ddr-base", 0x020, 0, 2, 0);
>
> Maybe we should set CLK_IS_CRITICAL on this one as well... in case the
> bootloader uses pll-periph for mbus, and none of the dram gates are enabled.
Ack.
> > +static SUNXI_CCU_GATE(hosc_clk, "hosc", "osc24M", 0x050, BIT(0), 0);
>
> Why the extra "hosc" clock here? You should probably just internalize "osc24M".
I'd prefer to model it as it is modelled in hardware: you have two
crystals, and then a gate within the CCU.
> > +static const char * const csi_parents[] = { "hosc", "pll-video0", "pll-video1",
> > + "pll-video0-2x", "pll-video1-2x" };
> > +static const u8 csi_table[] = { 0, 1, 2, 5, 6 };
> > +static SUNXI_CCU_M_WITH_MUX_TABLE_GATE(csi_clk, "csi",
> > + csi_parents, csi_table,
> > + 0x134, 0, 5, 24, 2, BIT(31), 0);
>
> Do you know if CSI needs to change the module clock?
Apparently not.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161121/bb431cdc/attachment.sig>
^ permalink raw reply
* [PATCH 1/2] MAINTAINERS: Update fpga-mgr entry with new linux-fpga mailing list
From: Moritz Fischer @ 2016-11-21 21:49 UTC (permalink / raw)
To: linux-arm-kernel
Update the fpga-mgr framework entry with new linux-fpga at vger.kernel.org
mailing list.
Signed-off-by: Moritz Fischer <mdf@kernel.org>
Cc: Alan Tull <atull@opensource.altera.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Michal Simek <monstr@monstr.eu>
Cc: S?ren Brinkmann <soren.brinkmann@xilinx.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-fpga at vger.kernel.org
Cc: linux-kernel at vger.kernel.org
---
Hi all,
Alan & me have been talking about this during Plumbers, so I went ahead
and worked on getting it all set up.
The list is now up and running & you can sign up if you're interested,
we'll be using it in parallel to LKML in the beginning I think.
I'm currently working on getting a list archive setup over at marc.info.
If you have comments or suggestions on alternatives, send me an email.
Cheers,
Moritz
---
MAINTAINERS | 1 +
1 file changed, 1 insertion(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 5e25ba1..19527a0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5005,6 +5005,7 @@ K: fmc_d.*register
FPGA MANAGER FRAMEWORK
M: Alan Tull <atull@opensource.altera.com>
R: Moritz Fischer <moritz.fischer@ettus.com>
+L: linux-fpga at vger.kernel.org
S: Maintained
F: drivers/fpga/
F: include/linux/fpga/fpga-mgr.h
--
2.7.4
^ permalink raw reply related
* [PATCH 2/2] MAINTAINERS: Add myself as co-maintainer to fpga mgr framework.
From: Moritz Fischer @ 2016-11-21 21:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479764942-14321-1-git-send-email-mdf@kernel.org>
Add myself as co-maintainer to fpga mgr framework.
Signed-off-by: Moritz Fischer <mdf@kernel.org>
Cc: Alan Tull <atull@opensource.altera.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-kernel at vger.kernel.org
Cc: linux-fpga at vger.kernel.org
---
Hi all,
Lately we've fallen behind a bit on reviewing patches lately.
'Behind the scenes', Alan has worked on getting a git tree setup
for us on kernel.org and I've worked on getting the mailing list setup.
The mailing list is up and running (see patch [1/2]) of this series.
Greg said he'd be happy to take pull requests from now on, so I hope
once we have figured out the exact workflow things will be much smoother
getting your patches merged into the fpga mgr framework.
Feel free to comment / speak up if you're not cool with this change.
Thanks,
Moritz
---
MAINTAINERS | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 19527a0..1856594 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5004,7 +5004,7 @@ K: fmc_d.*register
FPGA MANAGER FRAMEWORK
M: Alan Tull <atull@opensource.altera.com>
-R: Moritz Fischer <moritz.fischer@ettus.com>
+M: Moritz Fischer <moritz.fischer@ettus.com>
L: linux-fpga at vger.kernel.org
S: Maintained
F: drivers/fpga/
--
2.7.4
^ permalink raw reply related
* [PATCH 9/10] ARM: sun5i: Convert to CCU
From: Maxime Ripard @ 2016-11-21 22:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAGb2v64vOePTQ0a7cmSmimVrkYOxxRwRZnGbsC2ZQ4Su59oJaQ@mail.gmail.com>
On Sun, Nov 13, 2016 at 11:01:27PM +0800, Chen-Yu Tsai wrote:
> > compatible = "allwinner,sun5i-a13-tcon";
> > reg = <0x01c0c000 0x1000>;
> > interrupts = <44>;
> > - resets = <&tcon_ch0_clk 1>;
> > + resets = <&ccu RST_LCD>;
> > reset-names = "lcd";
> > - clocks = <&ahb_gates 36>,
> > - <&tcon_ch0_clk>,
> > - <&tcon_ch1_clk>;
> > + clocks = <&ccu CLK_AHB_LCD>,
> > + <&ccu CLK_TCON_CH0>,
> > + <&ccu CLK_TCON_CH1>;
>
> Just curious, is CLK_TCON_CH1_SCLK ever used?
Yes, this is an intermediate clock for the channel 1 clock.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161121/1222cbd5/attachment.sig>
^ permalink raw reply
* [PATCH 2/2] MAINTAINERS: Add myself as co-maintainer to fpga mgr framework.
From: Marek Vasut @ 2016-11-21 22:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479764942-14321-2-git-send-email-mdf@kernel.org>
On 11/21/2016 10:49 PM, Moritz Fischer wrote:
> Add myself as co-maintainer to fpga mgr framework.
>
> Signed-off-by: Moritz Fischer <mdf@kernel.org>
> Cc: Alan Tull <atull@opensource.altera.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-kernel at vger.kernel.org
> Cc: linux-fpga at vger.kernel.org
Acked-by: Marek Vasut <marex@denx.de>
> ---
> Hi all,
>
> Lately we've fallen behind a bit on reviewing patches lately.
>
> 'Behind the scenes', Alan has worked on getting a git tree setup
> for us on kernel.org and I've worked on getting the mailing list setup.
> The mailing list is up and running (see patch [1/2]) of this series.
>
> Greg said he'd be happy to take pull requests from now on, so I hope
> once we have figured out the exact workflow things will be much smoother
> getting your patches merged into the fpga mgr framework.
>
> Feel free to comment / speak up if you're not cool with this change.
>
> Thanks,
>
> Moritz
>
> ---
> MAINTAINERS | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 19527a0..1856594 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5004,7 +5004,7 @@ K: fmc_d.*register
>
> FPGA MANAGER FRAMEWORK
> M: Alan Tull <atull@opensource.altera.com>
> -R: Moritz Fischer <moritz.fischer@ettus.com>
> +M: Moritz Fischer <moritz.fischer@ettus.com>
> L: linux-fpga at vger.kernel.org
> S: Maintained
> F: drivers/fpga/
>
--
Best regards,
Marek Vasut
^ permalink raw reply
* [GIT PULL] Allwinner DT changes for 4.10
From: Maxime Ripard @ 2016-11-21 22:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAOesGMg=OQDtKkb4Mut4M1zAwM5CX3=YHxS7equO=Nx2a1tv1Q@mail.gmail.com>
On Mon, Nov 21, 2016 at 08:28:09AM -0800, Olof Johansson wrote:
> HI,
>
> On Mon, Nov 21, 2016 at 5:27 AM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > Hi Olof,
> >
> > On Fri, Nov 18, 2016 at 04:23:16PM -0800, Olof Johansson wrote:
> >> Hi,
> >>
> >> On Tue, Nov 15, 2016 at 09:41:22PM +0100, Maxime Ripard wrote:
> >> > Hi Arnd, Olof,
> >> >
> >> > Here is our pull request for the next merge window.
> >> >
> >> > Thanks!
> >> > Maxime
> >> >
> >> > The following changes since commit 1001354ca34179f3db924eb66672442a173147dc:
> >> >
> >> > Linux 4.9-rc1 (2016-10-15 12:17:50 -0700)
> >> >
> >> > are available in the git repository at:
> >> >
> >> > https://git.kernel.org/pub/scm/linux/kernel/git/mripard/linux.git tags/sunxi-dt-for-4.10
> >> >
> >> > for you to fetch changes up to e39a30cf736144814b0bddb3fff28fbbc2a8be0f:
> >> >
> >> > ARM: sunxi: Add the missing clocks to the pinctrl nodes (2016-11-15 18:49:47 +0100)
> >> >
> >> > ----------------------------------------------------------------
> >> > Allwinner DT additions for 4.10
> >> >
> >> > The usual bunch of DT additions, but most notably:
> >> > - A31 DRM driver
> >> > - A31 audio codec
> >> > - WiFi for the A80-Based boards and the CHIP
> >> > - Support for the NextThing Co CHIP Pro (the first board with NAND
> >> > enabled)
> >> > - New boards: NanoPi M1,
> >> >
> >> [...]
> >>
> >> > Maxime Ripard (16):
> >> > ARM: sun5i: a13-olinuxino: Enable VGA bridge
> >> > ARM: gr8: Add the UART3
> >> > ARM: gr8: Fix typo in the i2s mclk pin group
> >> > ARM: gr8: Add missing pwm channel 1 pin
> >> > ARM: gr8: Add UART2 pins
> >> > ARM: gr8: Add UART3 pins
> >> > ARM: gr8: Add CHIP Pro support
> >> > ARM: sun5i: chip: Enable Wi-Fi SDIO chip
> >> > ARM: sun5i: Rename A10s pins
> >> > ARM: sun5i: Add SPI2 pins
> >> > ARM: sun5i: Add RGB 565 LCD pins
> >> > ARM: sun5i: chip: Add optional buses
> >> > ARM: gr8: evb: Enable SPDIF
> >> > ARM: gr8: evb: Add i2s codec
> >> > ARM: sun8i: sina33: Enable USB gadget
> >> > ARM: sunxi: Add the missing clocks to the pinctrl nodes
> >> >
> >> [...]
> >>
> >> > arch/arm/boot/dts/Makefile | 1 +
> >> > arch/arm/boot/dts/ntc-gr8-chip-pro.dts | 266 +++++++++++++++++++++
> >> > arch/arm/boot/dts/ntc-gr8-evb.dts | 33 +++
> >> > arch/arm/boot/dts/ntc-gr8.dtsi | 47 +++-
> >> > arch/arm/boot/dts/sun4i-a10.dtsi | 3 +-
> >>
> >> NTC isn't the SoC manufacturer, and we try to keep the prefixes down to
> >> manufacturer to keep the namespace a little more manageable, even if
> >> we never got subdirectories setup as on arm64.
> >>
> >> I think this should probably be sun4i-a10-gr8 or sun4i-r8-gr8 as prefix?
> >
> > The users really expect a SoC from Nextthing, it's always been
> > marketed that way, the marking on the SoC also says so, etc. The fact
> > that it's been a design in cooperation with Allwinner, and that the
> > design is based on some earlier family is an implementation detail,
> > and I'd really like not for it to have the sun5i prefix, it's just
> > confusing.
>
> I don't care so much about what's printed on the top of the package, I
> care a lot more about what's on the insides. We've got a long
> tradition of not renaming things randomly when companies get acquired
> or renames themselves, and I think that same spirit is applicable
> here.
Yet, we called the imx23 and imx28 that way while it was really a
sigmatel design. And I'm pretty sure there's other examples too.
> Calling it an SoC is inaccurate as well, it's really a
> system-in-package. It's just a new way to integrate an SoC into a
> module to build boards out of. Compare to Octavo OSD335x, for example.
>
> System-in-package solutions are going to get more and more common, and
> it's going to become really chaotic if we expect to use the prefix of
> the company custom-ordering the package for each and every one of
> them.
This is indeed a SIP, but one with a custom SoC (or whatever the !RAM
part in a SIP is called) that was designed by Allwinner *and* Next
Thing Co., unlike the OSD335x that has a standard AM335x SoC in it.
> > And the ntc prefix has been asked for during the reviews...
>
> Having a link to that requeset/email would be helpful if you try to
> use it as an argument.
Yeah, it turns out it was off list... so it's a pretty weak one :)
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161121/a2e39d2c/attachment.sig>
^ permalink raw reply
* [PATCH V5 00/10] Add UEFI 2.6 and ACPI 6.1 updates for RAS on ARM64
From: Tyler Baicar @ 2016-11-21 22:35 UTC (permalink / raw)
To: linux-arm-kernel
When a memory error, CPU error, PCIe error, or other type of hardware error
that's covered by RAS occurs, firmware should populate the shared GHES memory
location with the proper GHES structures to notify the OS of the error.
For example, platforms that implement firmware first handling may implement
separate GHES sources for corrected errors and uncorrected errors. If the
error is an uncorrectable error, then the firmware will notify the OS
immediately since the error needs to be handled ASAP. The OS will then be able
to take the appropriate action needed such as offlining a page. If the error
is a corrected error, then the firmware will not interrupt the OS immediately.
Instead, the OS will see and report the error the next time it's GHES timer
expires. The kernel will first parse the GHES structures and report the errors
through the kernel logs and then notify the user space through RAS trace
events. This allows user space applications such as RAS Daemon to see the
errors and report them however the user desires. This patchset extends the
kernel functionality for RAS errors based on updates in the UEFI 2.6 and
ACPI 6.1 specifications.
An example flow from firmware to user space could be:
+---------------+
+-------->| |
| | GHES polling |--+
+-------------+ | source | | +---------------+ +------------+
| | +---------------+ | | Kernel GHES | | |
| Firmware | +-->| CPER AER and |-->| RAS trace |
| | +---------------+ | | EDAC drivers | | event |
+-------------+ | | | +---------------+ +------------+
| | GHES sci |--+
+-------->| source |
+---------------+
Add support for Generic Hardware Error Source (GHES) v2, which introduces the
capability for the OS to acknowledge the consumption of the error record
generated by the Reliability, Availability and Serviceability (RAS) controller.
This eliminates potential race conditions between the OS and the RAS controller.
Add support for the timestamp field added to the Generic Error Data Entry v3,
allowing the OS to log the time that the error is generated by the firmware,
rather than the time the error is consumed. This improves the correctness of
event sequences when analyzing error logs. The timestamp is added in
ACPI 6.1, reference Table 18-343 Generic Error Data Entry.
Add support for ARMv8 Common Platform Error Record (CPER) per UEFI 2.6
specification. ARMv8 specific processor error information is reported as part of
the CPER records. This provides more detail on for processor error logs. This
can help describe ARMv8 cache, tlb, and bus errors.
Synchronous External Abort (SEA) represents a specific processor error condition
in ARM systems. A handler is added to recognize SEA errors, and a notifier is
added to parse and report the errors before the process is killed. Refer to
section N.2.1.1 in the Common Platform Error Record appendix of the UEFI 2.6
specification.
Currently the kernel ignores CPER records that are unrecognized.
On the other hand, UEFI spec allows for non-standard (eg. vendor
proprietary) error section type in CPER (Common Platform Error Record),
as defined in section N2.3 of UEFI version 2.5. Therefore, user
is not able to see hardware error data of non-standard section.
If section Type field of Generic Error Data Entry is unrecognized,
prints out the raw data in dmesg buffer, and also adds a tracepoint
for reporting such hardware errors.
Currently even if an error status block's severity is fatal, the kernel
does not honor the severity level and panic. With the firmware first
model, the platform could inform the OS about a fatal hardware error
through the non-NMI GHES notification type. The OS should panic when a
hardware error record is received with this severity.
Add support to handle SEAs that occur while a KVM guest kernel is
running. Currently these are unsupported by the guest abort handling.
Depends on: [PATCH v14] acpi, apei, arm64: APEI initial support for aarch64.
https://lkml.org/lkml/2016/8/10/231
V5: Fix GHES goto logic for error conditions
Change ghes_do_read_ack to ghes_ack_error
Make sure data version check is >= 3
Use CPER helper functions in print functions
Make handle_guest_sea() dummy function static for arm
Add arm to subject line for KVM patch
V4: Add bit offset left shift to read_ack_write value
Make HEST generic and generic_v2 structures a union in the ghes structure
Move gdata v3 helper functions into ghes.h to avoid duplication
Reorder the timestamp print and avoid memcpy
Add helper functions for gdata size checking
Rename the SEA functions
Add helper function for GHES panics
Set fru_id to NULL UUID at variable declaration
Limit ARM trace event parameters to the needed structures
Reorder the ARM trace event variables to save space
Add comment for why we don't pass SEAs to the guest when it aborts
Move ARM trace event call into GHES driver instead of CPER
V3: Fix unmapped address to the read_ack_register in ghes.c
Add helper function to get the proper payload based on generic data entry
version
Move timestamp print to avoid changing function calls in cper.c
Remove patch "arm64: exception: handle instruction abort at current EL"
since the el1_ia handler is already added in 4.8
Add EFI and ARM64 dependencies for HAVE_ACPI_APEI_SEA
Add a new trace event for ARM type errors
Add support to handle KVM guest SEAs
V2: Add PSCI state print for the ARMv8 error type.
Separate timestamp year into year and century using BCD format.
Rebase on top of ACPICA 20160318 release and remove header file changes
in include/acpi/actbl1.h.
Add panic OS with fatal error status block patch.
Add processing of unrecognized CPER error section patches with updates
from previous comments. Original patches: https://lkml.org/lkml/2015/9/8/646
V1: https://lkml.org/lkml/2016/2/5/544
Jonathan (Zhixiong) Zhang (1):
acpi: apei: panic OS with fatal error status block
Tyler Baicar (9):
acpi: apei: read ack upon ghes record consumption
ras: acpi/apei: cper: generic error data entry v3 per ACPI 6.1
efi: parse ARMv8 processor error
arm64: exception: handle Synchronous External Abort
acpi: apei: handle SEA notification type for ARMv8
efi: print unrecognized CPER section
ras: acpi / apei: generate trace event for unrecognized CPER section
trace, ras: add ARM processor error trace event
arm/arm64: KVM: add guest SEA support
arch/arm/include/asm/kvm_arm.h | 1 +
arch/arm/include/asm/system_misc.h | 5 +
arch/arm/kvm/mmu.c | 18 ++-
arch/arm64/Kconfig | 1 +
arch/arm64/include/asm/kvm_arm.h | 1 +
arch/arm64/include/asm/system_misc.h | 15 +++
arch/arm64/mm/fault.c | 71 ++++++++++--
drivers/acpi/apei/Kconfig | 14 +++
drivers/acpi/apei/ghes.c | 188 ++++++++++++++++++++++++++++---
drivers/acpi/apei/hest.c | 7 +-
drivers/firmware/efi/cper.c | 210 ++++++++++++++++++++++++++++++++---
drivers/ras/ras.c | 2 +
include/acpi/ghes.h | 15 ++-
include/linux/cper.h | 84 ++++++++++++++
include/ras/ras_event.h | 100 +++++++++++++++++
15 files changed, 688 insertions(+), 44 deletions(-)
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
^ permalink raw reply
* [PATCH V5 01/10] acpi: apei: read ack upon ghes record consumption
From: Tyler Baicar @ 2016-11-21 22:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479767763-27532-1-git-send-email-tbaicar@codeaurora.org>
A RAS (Reliability, Availability, Serviceability) controller
may be a separate processor running in parallel with OS
execution, and may generate error records for consumption by
the OS. If the RAS controller produces multiple error records,
then they may be overwritten before the OS has consumed them.
The Generic Hardware Error Source (GHES) v2 structure
introduces the capability for the OS to acknowledge the
consumption of the error record generated by the RAS
controller. A RAS controller supporting GHESv2 shall wait for
the acknowledgment before writing a new error record, thus
eliminating the race condition.
Signed-off-by: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
Signed-off-by: Richard Ruigrok <rruigrok@codeaurora.org>
Signed-off-by: Tyler Baicar <tbaicar@codeaurora.org>
Signed-off-by: Naveen Kaje <nkaje@codeaurora.org>
---
drivers/acpi/apei/ghes.c | 49 +++++++++++++++++++++++++++++++++++++++++++++---
drivers/acpi/apei/hest.c | 7 +++++--
include/acpi/ghes.h | 5 ++++-
3 files changed, 55 insertions(+), 6 deletions(-)
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 60746ef..b79abc5 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -45,6 +45,7 @@
#include <linux/aer.h>
#include <linux/nmi.h>
+#include <acpi/actbl1.h>
#include <acpi/ghes.h>
#include <acpi/apei.h>
#include <asm/tlbflush.h>
@@ -79,6 +80,10 @@
((struct acpi_hest_generic_status *) \
((struct ghes_estatus_node *)(estatus_node) + 1))
+#define HEST_TYPE_GENERIC_V2(ghes) \
+ ((struct acpi_hest_header *)ghes->generic)->type == \
+ ACPI_HEST_TYPE_GENERIC_ERROR_V2
+
/*
* This driver isn't really modular, however for the time being,
* continuing to use module_param is the easiest way to remain
@@ -248,10 +253,18 @@ static struct ghes *ghes_new(struct acpi_hest_generic *generic)
ghes = kzalloc(sizeof(*ghes), GFP_KERNEL);
if (!ghes)
return ERR_PTR(-ENOMEM);
+
ghes->generic = generic;
+ if (HEST_TYPE_GENERIC_V2(ghes)) {
+ rc = apei_map_generic_address(
+ &ghes->generic_v2->read_ack_register);
+ if (rc)
+ goto err_free;
+ }
+
rc = apei_map_generic_address(&generic->error_status_address);
if (rc)
- goto err_free;
+ goto err_unmap_read_ack_addr;
error_block_length = generic->error_block_length;
if (error_block_length > GHES_ESTATUS_MAX_SIZE) {
pr_warning(FW_WARN GHES_PFX
@@ -263,13 +276,17 @@ static struct ghes *ghes_new(struct acpi_hest_generic *generic)
ghes->estatus = kmalloc(error_block_length, GFP_KERNEL);
if (!ghes->estatus) {
rc = -ENOMEM;
- goto err_unmap;
+ goto err_unmap_status_addr;
}
return ghes;
-err_unmap:
+err_unmap_status_addr:
apei_unmap_generic_address(&generic->error_status_address);
+err_unmap_read_ack_addr:
+ if (HEST_TYPE_GENERIC_V2(ghes))
+ apei_unmap_generic_address(
+ &ghes->generic_v2->read_ack_register);
err_free:
kfree(ghes);
return ERR_PTR(rc);
@@ -279,6 +296,9 @@ static void ghes_fini(struct ghes *ghes)
{
kfree(ghes->estatus);
apei_unmap_generic_address(&ghes->generic->error_status_address);
+ if (HEST_TYPE_GENERIC_V2(ghes))
+ apei_unmap_generic_address(
+ &ghes->generic_v2->read_ack_register);
}
static inline int ghes_severity(int severity)
@@ -648,6 +668,23 @@ static void ghes_estatus_cache_add(
rcu_read_unlock();
}
+static int ghes_ack_error(struct acpi_hest_generic_v2 *generic_v2)
+{
+ int rc;
+ u64 val = 0;
+
+ rc = apei_read(&val, &generic_v2->read_ack_register);
+ if (rc)
+ return rc;
+ val &= generic_v2->read_ack_preserve <<
+ generic_v2->read_ack_register.bit_offset;
+ val |= generic_v2->read_ack_write <<
+ generic_v2->read_ack_register.bit_offset;
+ rc = apei_write(val, &generic_v2->read_ack_register);
+
+ return rc;
+}
+
static int ghes_proc(struct ghes *ghes)
{
int rc;
@@ -660,6 +697,12 @@ static int ghes_proc(struct ghes *ghes)
ghes_estatus_cache_add(ghes->generic, ghes->estatus);
}
ghes_do_proc(ghes, ghes->estatus);
+
+ if (HEST_TYPE_GENERIC_V2(ghes)) {
+ rc = ghes_ack_error(ghes->generic_v2);
+ if (rc)
+ return rc;
+ }
out:
ghes_clear_estatus(ghes);
return 0;
diff --git a/drivers/acpi/apei/hest.c b/drivers/acpi/apei/hest.c
index 792a0d9..ef725a9 100644
--- a/drivers/acpi/apei/hest.c
+++ b/drivers/acpi/apei/hest.c
@@ -52,6 +52,7 @@ static const int hest_esrc_len_tab[ACPI_HEST_TYPE_RESERVED] = {
[ACPI_HEST_TYPE_AER_ENDPOINT] = sizeof(struct acpi_hest_aer),
[ACPI_HEST_TYPE_AER_BRIDGE] = sizeof(struct acpi_hest_aer_bridge),
[ACPI_HEST_TYPE_GENERIC_ERROR] = sizeof(struct acpi_hest_generic),
+ [ACPI_HEST_TYPE_GENERIC_ERROR_V2] = sizeof(struct acpi_hest_generic_v2),
};
static int hest_esrc_len(struct acpi_hest_header *hest_hdr)
@@ -146,7 +147,8 @@ static int __init hest_parse_ghes_count(struct acpi_hest_header *hest_hdr, void
{
int *count = data;
- if (hest_hdr->type == ACPI_HEST_TYPE_GENERIC_ERROR)
+ if (hest_hdr->type == ACPI_HEST_TYPE_GENERIC_ERROR ||
+ hest_hdr->type == ACPI_HEST_TYPE_GENERIC_ERROR_V2)
(*count)++;
return 0;
}
@@ -157,7 +159,8 @@ static int __init hest_parse_ghes(struct acpi_hest_header *hest_hdr, void *data)
struct ghes_arr *ghes_arr = data;
int rc, i;
- if (hest_hdr->type != ACPI_HEST_TYPE_GENERIC_ERROR)
+ if (hest_hdr->type != ACPI_HEST_TYPE_GENERIC_ERROR &&
+ hest_hdr->type != ACPI_HEST_TYPE_GENERIC_ERROR_V2)
return 0;
if (!((struct acpi_hest_generic *)hest_hdr)->enabled)
diff --git a/include/acpi/ghes.h b/include/acpi/ghes.h
index 720446c..68f088a 100644
--- a/include/acpi/ghes.h
+++ b/include/acpi/ghes.h
@@ -13,7 +13,10 @@
#define GHES_EXITING 0x0002
struct ghes {
- struct acpi_hest_generic *generic;
+ union {
+ struct acpi_hest_generic *generic;
+ struct acpi_hest_generic_v2 *generic_v2;
+ };
struct acpi_hest_generic_status *estatus;
u64 buffer_paddr;
unsigned long flags;
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
^ permalink raw reply related
* [PATCH V5 02/10] ras: acpi/apei: cper: generic error data entry v3 per ACPI 6.1
From: Tyler Baicar @ 2016-11-21 22:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479767763-27532-1-git-send-email-tbaicar@codeaurora.org>
Currently when a RAS error is reported it is not timestamped.
The ACPI 6.1 spec adds the timestamp field to the generic error
data entry v3 structure. The timestamp of when the firmware
generated the error is now being reported.
Signed-off-by: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
Signed-off-by: Richard Ruigrok <rruigrok@codeaurora.org>
Signed-off-by: Tyler Baicar <tbaicar@codeaurora.org>
Signed-off-by: Naveen Kaje <nkaje@codeaurora.org>
---
drivers/acpi/apei/ghes.c | 14 +++++++---
drivers/firmware/efi/cper.c | 62 +++++++++++++++++++++++++++++++++++----------
include/acpi/ghes.h | 10 ++++++++
include/linux/cper.h | 12 +++++++++
4 files changed, 80 insertions(+), 18 deletions(-)
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index b79abc5..9063d68 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -420,7 +420,8 @@ static void ghes_handle_memory_failure(struct acpi_hest_generic_data *gdata, int
int flags = -1;
int sec_sev = ghes_severity(gdata->error_severity);
struct cper_sec_mem_err *mem_err;
- mem_err = (struct cper_sec_mem_err *)(gdata + 1);
+
+ mem_err = acpi_hest_generic_data_payload(gdata);
if (!(mem_err->validation_bits & CPER_MEM_VALID_PA))
return;
@@ -450,14 +451,18 @@ static void ghes_do_proc(struct ghes *ghes,
{
int sev, sec_sev;
struct acpi_hest_generic_data *gdata;
+ uuid_le sec_type;
sev = ghes_severity(estatus->error_severity);
apei_estatus_for_each_section(estatus, gdata) {
sec_sev = ghes_severity(gdata->error_severity);
- if (!uuid_le_cmp(*(uuid_le *)gdata->section_type,
+ sec_type = *(uuid_le *)gdata->section_type;
+
+ if (!uuid_le_cmp(sec_type,
CPER_SEC_PLATFORM_MEM)) {
struct cper_sec_mem_err *mem_err;
- mem_err = (struct cper_sec_mem_err *)(gdata+1);
+
+ mem_err = acpi_hest_generic_data_payload(gdata);
ghes_edac_report_mem_error(ghes, sev, mem_err);
arch_apei_report_mem_error(sev, mem_err);
@@ -467,7 +472,8 @@ static void ghes_do_proc(struct ghes *ghes,
else if (!uuid_le_cmp(*(uuid_le *)gdata->section_type,
CPER_SEC_PCIE)) {
struct cper_sec_pcie *pcie_err;
- pcie_err = (struct cper_sec_pcie *)(gdata+1);
+
+ pcie_err = acpi_hest_generic_data_payload(gdata);
if (sev == GHES_SEV_RECOVERABLE &&
sec_sev == GHES_SEV_RECOVERABLE &&
pcie_err->validation_bits & CPER_PCIE_VALID_DEVICE_ID &&
diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
index d425374..7e2439e 100644
--- a/drivers/firmware/efi/cper.c
+++ b/drivers/firmware/efi/cper.c
@@ -32,6 +32,9 @@
#include <linux/acpi.h>
#include <linux/pci.h>
#include <linux/aer.h>
+#include <linux/printk.h>
+#include <linux/bcd.h>
+#include <acpi/ghes.h>
#define INDENT_SP " "
@@ -386,13 +389,37 @@ static void cper_print_pcie(const char *pfx, const struct cper_sec_pcie *pcie,
pfx, pcie->bridge.secondary_status, pcie->bridge.control);
}
+static void cper_estatus_print_section_v300(const char *pfx,
+ const struct acpi_hest_generic_data_v300 *gdata)
+{
+ __u8 hour, min, sec, day, mon, year, century, *timestamp;
+
+ if (gdata->validation_bits & ACPI_HEST_GEN_VALID_TIMESTAMP) {
+ timestamp = (__u8 *)&(gdata->time_stamp);
+ sec = bcd2bin(timestamp[0]);
+ min = bcd2bin(timestamp[1]);
+ hour = bcd2bin(timestamp[2]);
+ day = bcd2bin(timestamp[4]);
+ mon = bcd2bin(timestamp[5]);
+ year = bcd2bin(timestamp[6]);
+ century = bcd2bin(timestamp[7]);
+ printk("%stime: %7s %02d%02d-%02d-%02d %02d:%02d:%02d\n", pfx,
+ 0x01 & *(timestamp + 3) ? "precise" : "", century,
+ year, mon, day, hour, min, sec);
+ }
+}
+
static void cper_estatus_print_section(
- const char *pfx, const struct acpi_hest_generic_data *gdata, int sec_no)
+ const char *pfx, struct acpi_hest_generic_data *gdata, int sec_no)
{
uuid_le *sec_type = (uuid_le *)gdata->section_type;
__u16 severity;
char newpfx[64];
+ if (acpi_hest_generic_data_version(gdata) >= 3)
+ cper_estatus_print_section_v300(pfx,
+ (const struct acpi_hest_generic_data_v300 *)gdata);
+
severity = gdata->error_severity;
printk("%s""Error %d, type: %s\n", pfx, sec_no,
cper_severity_str(severity));
@@ -403,14 +430,18 @@ static void cper_estatus_print_section(
snprintf(newpfx, sizeof(newpfx), "%s%s", pfx, INDENT_SP);
if (!uuid_le_cmp(*sec_type, CPER_SEC_PROC_GENERIC)) {
- struct cper_sec_proc_generic *proc_err = (void *)(gdata + 1);
+ struct cper_sec_proc_generic *proc_err;
+
+ proc_err = acpi_hest_generic_data_payload(gdata);
printk("%s""section_type: general processor error\n", newpfx);
if (gdata->error_data_length >= sizeof(*proc_err))
cper_print_proc_generic(newpfx, proc_err);
else
goto err_section_too_small;
} else if (!uuid_le_cmp(*sec_type, CPER_SEC_PLATFORM_MEM)) {
- struct cper_sec_mem_err *mem_err = (void *)(gdata + 1);
+ struct cper_sec_mem_err *mem_err;
+
+ mem_err = acpi_hest_generic_data_payload(gdata);
printk("%s""section_type: memory error\n", newpfx);
if (gdata->error_data_length >=
sizeof(struct cper_sec_mem_err_old))
@@ -419,7 +450,9 @@ static void cper_estatus_print_section(
else
goto err_section_too_small;
} else if (!uuid_le_cmp(*sec_type, CPER_SEC_PCIE)) {
- struct cper_sec_pcie *pcie = (void *)(gdata + 1);
+ struct cper_sec_pcie *pcie;
+
+ pcie = acpi_hest_generic_data_payload(gdata);
printk("%s""section_type: PCIe error\n", newpfx);
if (gdata->error_data_length >= sizeof(*pcie))
cper_print_pcie(newpfx, pcie, gdata);
@@ -438,7 +471,7 @@ void cper_estatus_print(const char *pfx,
const struct acpi_hest_generic_status *estatus)
{
struct acpi_hest_generic_data *gdata;
- unsigned int data_len, gedata_len;
+ unsigned int data_len;
int sec_no = 0;
char newpfx[64];
__u16 severity;
@@ -451,12 +484,12 @@ void cper_estatus_print(const char *pfx,
printk("%s""event severity: %s\n", pfx, cper_severity_str(severity));
data_len = estatus->data_length;
gdata = (struct acpi_hest_generic_data *)(estatus + 1);
+
snprintf(newpfx, sizeof(newpfx), "%s%s", pfx, INDENT_SP);
- while (data_len >= sizeof(*gdata)) {
- gedata_len = gdata->error_data_length;
+
+ while (data_len >= acpi_hest_generic_data_size(gdata)) {
cper_estatus_print_section(newpfx, gdata, sec_no);
- data_len -= gedata_len + sizeof(*gdata);
- gdata = (void *)(gdata + 1) + gedata_len;
+ gdata = acpi_hest_generic_data_next(gdata);
sec_no++;
}
}
@@ -486,12 +519,13 @@ int cper_estatus_check(const struct acpi_hest_generic_status *estatus)
return rc;
data_len = estatus->data_length;
gdata = (struct acpi_hest_generic_data *)(estatus + 1);
- while (data_len >= sizeof(*gdata)) {
- gedata_len = gdata->error_data_length;
- if (gedata_len > data_len - sizeof(*gdata))
+
+ while (data_len >= acpi_hest_generic_data_size(gdata)) {
+ gedata_len = acpi_hest_generic_data_error_length(gdata);
+ if (gedata_len > data_len - acpi_hest_generic_data_size(gdata))
return -EINVAL;
- data_len -= gedata_len + sizeof(*gdata);
- gdata = (void *)(gdata + 1) + gedata_len;
+ data_len -= gedata_len + acpi_hest_generic_data_size(gdata);
+ gdata = acpi_hest_generic_data_next(gdata);
}
if (data_len)
return -EINVAL;
diff --git a/include/acpi/ghes.h b/include/acpi/ghes.h
index 68f088a..56b9679 100644
--- a/include/acpi/ghes.h
+++ b/include/acpi/ghes.h
@@ -73,3 +73,13 @@ static inline void ghes_edac_unregister(struct ghes *ghes)
{
}
#endif
+
+#define acpi_hest_generic_data_version(gdata) \
+ (gdata->revision >> 8)
+
+static inline void *acpi_hest_generic_data_payload(struct acpi_hest_generic_data *gdata)
+{
+ return acpi_hest_generic_data_version(gdata) >= 3 ?
+ (void *)(((struct acpi_hest_generic_data_v300 *)(gdata)) + 1) :
+ gdata + 1;
+}
diff --git a/include/linux/cper.h b/include/linux/cper.h
index dcacb1a..13ea41c 100644
--- a/include/linux/cper.h
+++ b/include/linux/cper.h
@@ -255,6 +255,18 @@ enum {
#define CPER_PCIE_SLOT_SHIFT 3
+#define acpi_hest_generic_data_error_length(gdata) \
+ (((struct acpi_hest_generic_data *)(gdata))->error_data_length)
+#define acpi_hest_generic_data_size(gdata) \
+ ((acpi_hest_generic_data_version(gdata) >= 3) ? \
+ sizeof(struct acpi_hest_generic_data_v300) : \
+ sizeof(struct acpi_hest_generic_data))
+#define acpi_hest_generic_data_record_size(gdata) \
+ (acpi_hest_generic_data_size(gdata) + \
+ acpi_hest_generic_data_error_length(gdata))
+#define acpi_hest_generic_data_next(gdata) \
+ ((void *)(gdata) + acpi_hest_generic_data_record_size(gdata))
+
/*
* All tables and structs must be byte-packed to match CPER
* specification, since the tables are provided by the system BIOS
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
^ 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