* [PATCH V6 3/5] PCI: thunder-pem: Allow to probe PEM-specific register range for ACPI case
From: Lorenzo Pieralisi @ 2016-09-23 10:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160922221042.GA1514@localhost>
[+ Zhang Rui]
On Thu, Sep 22, 2016 at 05:10:42PM -0500, Bjorn Helgaas wrote:
> On Thu, Sep 22, 2016 at 01:31:01PM -0500, Bjorn Helgaas wrote:
> > On Thu, Sep 22, 2016 at 01:44:46PM +0100, Lorenzo Pieralisi wrote:
> > > On Thu, Sep 22, 2016 at 11:10:13AM +0000, Gabriele Paoloni wrote:
> > > > Hi Lorenzo, Bjorn
> > > >
> > > > > -----Original Message-----
> > > > > From: Lorenzo Pieralisi [mailto:lorenzo.pieralisi at arm.com]
> > > > > Sent: 22 September 2016 10:50
> > > > > To: Bjorn Helgaas
> > > > > Cc: Ard Biesheuvel; Tomasz Nowicki; David Daney; Will Deacon; Catalin
> > > > > Marinas; Rafael Wysocki; Arnd Bergmann; Hanjun Guo; Sinan Kaya;
> > > > > Jayachandran C; Christopher Covington; Duc Dang; Robert Richter; Marcin
> > > > > Wojtas; Liviu Dudau; Wangyijing; Mark Salter; linux-
> > > > > pci at vger.kernel.org; linux-arm-kernel at lists.infradead.org; Linaro ACPI
> > > > > Mailman List; Jon Masters; Andrea Gallo; Jeremy Linton; liudongdong
> > > > > (C); Gabriele Paoloni; Jeff Hugo; linux-acpi at vger.kernel.org; linux-
> > > > > kernel at vger.kernel.org; Rafael J. Wysocki
> > > > > Subject: Re: [PATCH V6 3/5] PCI: thunder-pem: Allow to probe PEM-
> > > > > specific register range for ACPI case
> > > > >
> > > > > On Wed, Sep 21, 2016 at 01:04:57PM -0500, Bjorn Helgaas wrote:
> > > > > > On Wed, Sep 21, 2016 at 03:05:49PM +0100, Lorenzo Pieralisi wrote:
> > > > > > > On Tue, Sep 20, 2016 at 02:17:44PM -0500, Bjorn Helgaas wrote:
> > > > > > > > On Tue, Sep 20, 2016 at 04:09:25PM +0100, Ard Biesheuvel wrote:
> > > > > > >
> > > > > > > [...]
> > > > > > >
> > > > > > > > > None of these platforms can be fixed entirely in software, and
> > > > > given
> > > > > > > > > that we will not be adding quirks for new broken hardware, we
> > > > > should
> > > > > > > > > ask ourselves whether having two versions of a quirk, i.e., one
> > > > > for
> > > > > > > > > broken hardware + currently shipping firmware, and one for the
> > > > > same
> > > > > > > > > broken hardware with fixed firmware is really an improvement
> > > > > over what
> > > > > > > > > has been proposed here.
> > > > > > > >
> > > > > > > > We're talking about two completely different types of quirks:
> > > > > > > >
> > > > > > > > 1) MCFG quirks to use memory-mapped config space that doesn't
> > > > > quite
> > > > > > > > conform to the ECAM model in the PCIe spec, and
> > > > > > > >
> > > > > > > > 2) Some yet-to-be-determined method to describe address space
> > > > > > > > consumed by a bridge.
> > > > > > > >
> > > > > > > > The first two patches of this series are a nice implementation
> > > > > for 1).
> > > > > > > > The third patch (ThunderX-specific) is one possibility for 2),
> > > > > but I
> > > > > > > > don't like it because there's no way for generic software like
> > > > > the
> > > > > > > > ACPI core to discover these resources.
> > > > > > >
> > > > > > > Ok, so basically this means that to implement (2) we need to assign
> > > > > > > some sort of _HID to these quirky PCI bridges (so that we know what
> > > > > > > device they represent and we can retrieve their _CRS). I take from
> > > > > > > this discussion that the goal is to make sure that all non-config
> > > > > > > resources have to be declared through _CRS device objects, which is
> > > > > > > fine but that requires a FW update (unless we can fabricate ACPI
> > > > > > > devices and corresponding _CRS in the kernel whenever we match a
> > > > > > > given MCFG table signature).
> > > > > >
> > > > > > All resources consumed by ACPI devices should be declared through
> > > > > > _CRS. If you want to fabricate ACPI devices or _CRS via kernel
> > > > > > quirks, that's fine with me. This could be triggered via MCFG
> > > > > > signature, DMI info, host bridge _HID, etc.
> > > > >
> > > > > I think the PNP quirk approach + PNP0c02 resource put forward by Gab
> > > > > is enough.
> > > >
> > > > Great thanks as we take a final decision I will ask Dogndgong to submit
> > > > another RFC based on this approach
> > > >
> > > > >
> > > > > > > We discussed this already and I think we should make a decision:
> > > > > > >
> > > > > > > http://lists.infradead.org/pipermail/linux-arm-kernel/2016-
> > > > > March/414722.html
> > > > > > >
> > > > > > > > > > I'd like to step back and come up with some understanding of
> > > > > how
> > > > > > > > > > non-broken firmware *should* deal with this issue. Then, if
> > > > > we *do*
> > > > > > > > > > work around this particular broken firmware in the kernel, it
> > > > > would be
> > > > > > > > > > nice to do it in a way that fits in with that understanding.
> > > > > > > > > >
> > > > > > > > > > For example, if a companion ACPI device is the preferred
> > > > > solution, an
> > > > > > > > > > ACPI quirk could fabricate a device with the required
> > > > > resources. That
> > > > > > > > > > would address the problem closer to the source and make it
> > > > > more likely
> > > > > > > > > > that the rest of the system will work correctly: /proc/iomem
> > > > > could
> > > > > > > > > > make sense, things that look at _CRS generically would work
> > > > > (e.g,
> > > > > > > > > > /sys/, an admittedly hypothetical "lsacpi", etc.)
> > > > > > > > > >
> > > > > > > > > > Hard-coding stuff in drivers is a point solution that doesn't
> > > > > provide
> > > > > > > > > > any guidance for future platforms and makes it likely that
> > > > > the hack
> > > > > > > > > > will get copied into even more drivers.
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > OK, I see. But the guidance for future platforms should be 'do
> > > > > not
> > > > > > > > > rely on quirks', and what I am arguing here is that the more we
> > > > > polish
> > > > > > > > > up this code and make it clean and reusable, the more likely it
> > > > > is
> > > > > > > > > that will end up getting abused by new broken hardware that we
> > > > > set out
> > > > > > > > > to reject entirely in the first place.
> > > > > > > > >
> > > > > > > > > So of course, if the quirk involves claiming resources, let's
> > > > > make
> > > > > > > > > sure that this occurs in the cleanest and most compliant way
> > > > > possible.
> > > > > > > > > But any factoring/reuse concerns other than for the current
> > > > > crop of
> > > > > > > > > broken hardware should be avoided imo.
> > > > > > > >
> > > > > > > > If future hardware is completely ECAM-compliant and we don't need
> > > > > any
> > > > > > > > more MCFG quirks, that would be great.
> > > > > > >
> > > > > > > Yes.
> > > > > > >
> > > > > > > > But we'll still need to describe that memory-mapped config space
> > > > > > > > somewhere. If that's done with PNP0C02 or similar devices (as is
> > > > > done
> > > > > > > > on my x86 laptop), we'd be all set.
> > > > > > >
> > > > > > > I am not sure I understand what you mean here. Are you referring
> > > > > > > to MCFG regions reported as PNP0c02 resources through its _CRS ?
> > > > > >
> > > > > > Yes. PCI Firmware Spec r3.0, Table 4-2, note 2 says address ranges
> > > > > > reported via MCFG or _CBA should be reserved by _CRS of a PNP0C02
> > > > > > device.
> > > > >
> > > > > Ok, that's agreed. It goes without saying that since you are quoting
> > > > > the PCI spec, if FW fails to report MCFG regions in a PNP0c02 device
> > > > > _CRS I will consider that a FW bug.
> > > > >
> > > > > > > IIUC PNP0C02 is a reservation mechanism, but it does not help us
> > > > > > > associate its _CRS to a specific PCI host bridge instance, right ?
> > > > > >
> > > > > > Gab proposed a hierarchy that *would* associate a PNP0C02 device with
> > > > > > a PCI bridge:
> > > > > >
> > > > > > Device (PCI1) {
> > > > > > Name (_HID, "HISI0080") // PCI Express Root Bridge
> > > > > > Name (_CID, "PNP0A03") // Compatible PCI Root Bridge
> > > > > > Method (_CRS, 0, Serialized) { // Root complex resources
> > > > > (windows) }
> > > > > > Device (RES0) {
> > > > > > Name (_HID, "HISI0081") // HiSi PCIe RC config base address
> > > > > > Name (_CID, "PNP0C02") // Motherboard reserved resource
> > > > > > Name (_CRS, ResourceTemplate () { ... }
> > > > > > }
> > > > > > }
> > > > > >
> > > > > > That's a possibility. The PCI Firmware Spec suggests putting RES0 at
> > > > > > the root (under \_SB), but I don't know why.
> > > > > >
> > > > > > Putting it at the root means we couldn't generically associate it
> > > > > with
> > > > > > a bridge, although I could imagine something like this:
> > > > > >
> > > > > > Device (RES1) {
> > > > > > Name (_HID, "HISI0081") // HiSi PCIe RC config base address
> > > > > > Name (_CID, "PNP0C02") // Motherboard reserved resource
> > > > > > Name (_CRS, ResourceTemplate () { ... }
> > > > > > Method (BRDG) { "PCI1" } // hand-wavy ASL
> > > > > > }
> > > > > > Device (PCI1) {
> > > > > > Name (_HID, "HISI0080") // PCI Express Root Bridge
> > > > > > Name (_CID, "PNP0A03") // Compatible PCI Root Bridge
> > > > > > Method (_CRS, 0, Serialized) { // Root complex resources
> > > > > (windows) }
> > > > > > }
> > > > > >
> > > > > > Where you could search PNP0C02 devices for a cookie that matched the
> > > > > > host bridge.o
> > > > >
> > > > > Ok, I am fine with both and I think we are converging, but the way
> > > > > to solve this problem has to be uniform for all ARM partners (and
> > > > > not only ARM). Two points here:
> > > > >
> > > > > 1) Adding a device/subdevice allows people to add a _CRS reporting the
> > > > > non-window bridge resources. Fine. It also allows people to chuck in
> > > > > there all sorts of _DSD properties to describe their PCI host bridge
> > > > > as it is done with DT properties (those _DSD can contain eg clocks
> > > > > etc.), this may be tempting (so that they can reuse the same DT
> > > > > driver and do not have to update their firmware) but I want to be
> > > > > clear here: that must not happen. So, a subdevice with a _CRS to
> > > > > report resources, yes, but it will stop there.
> > > > > 2) It is unclear to me how to formalize the above. People should not
> > > > > write FW by reading the PCI mailing list, so these guidelines have
> > > > > to
> > > > > be written, somehow. I do not want to standardize quirks, I want
> > > > > to prevent random ACPI table content, which is different.
> > > > > Should I report this to the ACPI spec working group ? If we do
> > > > > not do that everyone will go solve this problem as they deem fit.
> > > > >
> > > >
> > > > Do we really need to formalize this?
> > > >
> > > > As we discussed in the Linaro call at the moment we have few vendors
> > > > that need quirks and we want to avoid promoting/accepting quirks for
> > > > the future.
> > > >
> > > > At the time of the call I think we decided to informally accept a set
> > > > of quirks for the current platforms and reject any other quirk coming
> > > > after a certain date/kernel version (this to be decided).
> > > >
> > > > I am not sure if there is a way to document/formalize a temporary
> > > > exception from the rule...
> > >
> > > - (1) will be enforced.
> >
> > I'm not sure it's necessary or possible to enforce a "no future
> > quirks" rule. For one thing, there's already a pretty strong
> > incentive to avoid quirks: if your hardware doesn't require quirks,
> > it works with OSes already in the field.
> >
> > MCFG quirks allow us to use the generic ACPI pci_root.c driver even if
> > the hardware doesn't support ECAM quite according to the spec.
> >
> > PNP0C02 usage is a workaround for the failure of the Consumer/Producer
> > bit. PNP0C02 quirks compensate for firmware that doesn't describe
> > resource usage accurately. It's possible the ACPI spec folks could
> > come up with a better Consumer/Producer workaround, if that's needed.
> > Apparently x86 hasn't needed it yet.
> >
> > If people add _DSD methods for clocks or whatnot, the hardware won't
> > work with the generic pci_root.c driver, so there's already an
> > incentive for avoiding them. x86 has managed without such methods;
> > arm64 should be able to do the same.
>
> Re-reading this, I'm afraid my response sounds a little dismissive,
> and I feel like I'm missing some important information. So I
> apologize if I missed your whole point, Lorenzo.
No you are spot on, I just wanted to emphasize, given that we are
adding an _HID and a subdevice, that developer should not be tempted
to use it to match against a PCI host driver to reuse the DT code,
we should not use the quirk mechanism as a backdoor to re-using DT
drivers in ACPI context.
Anyway, there is a review process to spot these possible misuses,
mine was just a heads-up, quirks will happen, I just do not want
to wreak the standard ACPI PCI firmware model to support them.
Given that there are already PNP0c02 bindings out there where the
PNP0c02 is used as in Gab's example:
https://patchwork.kernel.org/patch/4757111/
I think the only pending question I have is whether we are allowed
to define a PNP0A03 subdevice with a _CRS resource space that is
not contained in its parent _CRS, if we answer this question I
think we are done.
I will raise the PNP0c02 usage issue with the ASWG anyway.
Thanks !
Lorenzo
^ permalink raw reply
* [PATCH v2] gpio: Added zynq specific check for special pins on bank zero
From: Nava kishore Manne @ 2016-09-23 10:05 UTC (permalink / raw)
To: linux-arm-kernel
From: Nava kishore Manne <nava.manne@xilinx.com>
This patch adds zynq specific check for bank 0 pins 7 and 8
are special and cannot be used as inputs
Signed-off-by: Nava kishore Manne <navam@xilinx.com>
Acked-by: Michal Simek <michal.simek@xilinx.com>
---
Changes for v2:
-Removed un-used quirks for zynqmp.
drivers/gpio/gpio-zynq.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/gpio/gpio-zynq.c b/drivers/gpio/gpio-zynq.c
index e72794e..2b689e8 100644
--- a/drivers/gpio/gpio-zynq.c
+++ b/drivers/gpio/gpio-zynq.c
@@ -96,6 +96,9 @@
/* GPIO upper 16 bit mask */
#define ZYNQ_GPIO_UPPER_MASK 0xFFFF0000
+/* For GPIO quirks */
+#define ZYNQ_GPIO BIT(0)
+
/**
* struct zynq_gpio - gpio device private data structure
* @chip: instance of the gpio_chip
@@ -122,6 +125,7 @@ struct zynq_gpio {
*/
struct zynq_platform_data {
const char *label;
+ u32 quirks;
u16 ngpio;
int max_bank;
int bank_min[ZYNQMP_GPIO_MAX_BANK];
@@ -238,13 +242,19 @@ static void zynq_gpio_set_value(struct gpio_chip *chip, unsigned int pin,
static int zynq_gpio_dir_in(struct gpio_chip *chip, unsigned int pin)
{
u32 reg;
+ bool is_zynq_gpio;
unsigned int bank_num, bank_pin_num;
struct zynq_gpio *gpio = gpiochip_get_data(chip);
+ is_zynq_gpio = gpio->p_data->quirks & ZYNQ_GPIO;
zynq_gpio_get_bank_pin(pin, &bank_num, &bank_pin_num, gpio);
- /* bank 0 pins 7 and 8 are special and cannot be used as inputs */
- if (bank_num == 0 && (bank_pin_num == 7 || bank_pin_num == 8))
+ /*
+ * On zynq bank 0 pins 7 and 8 are special and cannot be used
+ * as inputs.
+ */
+ if (is_zynq_gpio && bank_num == 0 &&
+ (bank_pin_num == 7 || bank_pin_num == 8))
return -EINVAL;
/* clear the bit in direction mode reg to set the pin as input */
@@ -627,6 +637,7 @@ static const struct zynq_platform_data zynqmp_gpio_def = {
static const struct zynq_platform_data zynq_gpio_def = {
.label = "zynq_gpio",
+ .quirks = ZYNQ_GPIO,
.ngpio = ZYNQ_GPIO_NR_GPIOS,
.max_bank = ZYNQ_GPIO_MAX_BANK,
.bank_min[0] = ZYNQ_GPIO_BANK0_PIN_MIN(),
--
2.1.2
^ permalink raw reply related
* [PATCH v2 2/3] arm64: kgdb: prevent kgdb from being invoked recursively
From: Will Deacon @ 2016-09-23 10:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160923073327.9657-3-takahiro.akashi@linaro.org>
On Fri, Sep 23, 2016 at 04:33:26PM +0900, AKASHI Takahiro wrote:
> If a breakpoint is set in an interrupt-sensitive place,
> like gic_handle_irq(), a debug exception can be triggerred recursively.
> We will see the following message:
>
> KGDB: re-enter error: breakpoint removed ffffffc000081258
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 650 at kernel/debug/debug_core.c:435
> kgdb_handle_exception+0x1dc/0x1f4()
> Modules linked in:
> CPU: 0 PID: 650 Comm: sh Not tainted 3.17.0-rc2+ #177
> Call trace:
> [<ffffffc000087fac>] dump_backtrace+0x0/0x130
> [<ffffffc0000880ec>] show_stack+0x10/0x1c
> [<ffffffc0004d683c>] dump_stack+0x74/0xb8
> [<ffffffc0000ab824>] warn_slowpath_common+0x8c/0xb4
> [<ffffffc0000ab90c>] warn_slowpath_null+0x14/0x20
> [<ffffffc000121bfc>] kgdb_handle_exception+0x1d8/0x1f4
> [<ffffffc000092ffc>] kgdb_brk_fn+0x18/0x28
> [<ffffffc0000821c8>] brk_handler+0x9c/0xe8
> [<ffffffc0000811e8>] do_debug_exception+0x3c/0xac
> Exception stack(0xffffffc07e027650 to 0xffffffc07e027770)
> ...
> [<ffffffc000083cac>] el1_dbg+0x14/0x68
> [<ffffffc00012178c>] kgdb_cpu_enter+0x464/0x5c0
> [<ffffffc000121bb4>] kgdb_handle_exception+0x190/0x1f4
> [<ffffffc000092ffc>] kgdb_brk_fn+0x18/0x28
> [<ffffffc0000821c8>] brk_handler+0x9c/0xe8
> [<ffffffc0000811e8>] do_debug_exception+0x3c/0xac
> Exception stack(0xffffffc07e027ac0 to 0xffffffc07e027be0)
> ...
> [<ffffffc000083cac>] el1_dbg+0x14/0x68
> [<ffffffc00032e4b4>] __handle_sysrq+0x11c/0x190
> [<ffffffc00032e93c>] write_sysrq_trigger+0x4c/0x60
> [<ffffffc0001e7d58>] proc_reg_write+0x54/0x84
> [<ffffffc000192fa4>] vfs_write+0x98/0x1c8
> [<ffffffc0001939b0>] SyS_write+0x40/0xa0
>
> When some interrupt occurs, a breakpoint at gic_handle_irq() invokes kgdb.
> Kgdb then calls kgdb_roundup_cpus() to sync with other cpus. Current
> kgdb_roundup_cpus() unmasks interrupts temporarily to use
> smp_call_function(). This eventually allows another interrupt to occur and
> likely results in hitting a breakpoint at gic_handle_irq() again since
> a debug exception is always enabled in el1_irq.
>
> We can avoid this issue by specifying "nokgdbroundup" in a kernel command
> line, but this will also leave other cpus be in unknown state in terms of
> kgdb, and may result in interfering with kgdb.
>
> This patch re-implements kgdb_roundup_cpus() so that we won't have to
> enable interrupts there by using irq_work.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Jason Wessel <jason.wessel@windriver.com>
> Cc: <stable@vger.kernel.org> # 3.15-
> ---
> arch/arm64/kernel/kgdb.c | 32 ++++++++++++++++++++++++++++----
> 1 file changed, 28 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c
> index afe5f90..59c4aec 100644
> --- a/arch/arm64/kernel/kgdb.c
> +++ b/arch/arm64/kernel/kgdb.c
> @@ -19,10 +19,13 @@
> * along with this program. If not, see <http://www.gnu.org/licenses/>.
> */
>
> +#include <linux/cpumask.h>
> #include <linux/irq.h>
> +#include <linux/irq_work.h>
> #include <linux/kdebug.h>
> #include <linux/kgdb.h>
> #include <linux/kprobes.h>
> +#include <linux/percpu.h>
> #include <asm/traps.h>
>
> struct dbg_reg_def_t dbg_reg_def[DBG_MAX_REG_NUM] = {
> @@ -106,6 +109,8 @@ struct dbg_reg_def_t dbg_reg_def[DBG_MAX_REG_NUM] = {
> { "fpcr", 4, -1 },
> };
>
> +static DEFINE_PER_CPU(struct irq_work, kgdb_irq_work);
> +
> char *dbg_get_reg(int regno, void *mem, struct pt_regs *regs)
> {
> if (regno >= DBG_MAX_REG_NUM || regno < 0)
> @@ -258,16 +263,27 @@ static struct step_hook kgdb_step_hook = {
> .fn = kgdb_step_brk_fn
> };
>
> -static void kgdb_call_nmi_hook(void *ignored)
> +static void kgdb_roundup_hook(struct irq_work *work)
> {
> kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs());
> }
>
> void kgdb_roundup_cpus(unsigned long flags)
> {
> - local_irq_enable();
> - smp_call_function(kgdb_call_nmi_hook, NULL, 0);
> - local_irq_disable();
> + int cpu;
> + struct cpumask mask;
> + struct irq_work *work;
> +
> + mask = *cpu_online_mask;
Are you sure you want to do this on the stack? Assuming it's safe to
allocate here, why not use cpumask_var_t?
Will
^ permalink raw reply
* [PATCH V3 2/4] ARM64 LPC: LPC driver implementation on Hip06
From: Arnd Bergmann @ 2016-09-23 9:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <57E40665.8080005@gmail.com>
On Friday, September 23, 2016 12:27:17 AM CEST zhichang.yuan wrote:
> For this patch sketch, I have a question.
> Do we call pci_address_to_pio in arch_of_address_to_pio to get the
> corresponding logical IO port
> for LPC??
No, of course not, that would be silly:
The argument to pci_address_to_pio() is a phys_addr_t, and we we don't
have one because there is no address associated with your PIO, that
is the entire point of your driver!
Also, we already know the mapping because this is what the inb/outb
workaround is looking at, so there is absolutely no reason to call it
either.
> If we don't, it seems the LPC specific IO address will conflict with PCI
> host bridges' logical IO.
>
> Supposed our LPC populated the IO range from 0x100 to 0x3FF( this is
> normal for ISA similar
> devices), after arch_of_address_to_pio(), the r->start will be set as
> 0x100, r->end will be set as
> 0x3FF. And if there is one PCI host bridge who request a IO window size
> over 0x400 at the same
> time, the corresponding r->start and r->end will be set as 0x0, 0x3FF
> after of_address_to_resource
> for this host bridge. Then the IO conflict happens.
You would still need to reserve some space in the io_range_list
to avoid possible conflicts, which is a bit ugly with the current
definition of pci_register_io_range, but I'm sure can be done.
One way I can think of would be to change pci_register_io_range()
to just return the logical port number directly (it already
knows it!), and pass an invalid physical address (e.g.
#define ISA_WORKAROUND_IO_PORT_WINDOW -0x10000) into it for
invalid translations.
Another alternative that just occurred to me would be to move
the pci_address_to_pio() call from __of_address_to_resource()
into of_bus_pci_translate() and then do the special handling
for the ISA/LPC bus in of_bus_isa_translate().
Arnd
^ permalink raw reply
* [PATCH v6 1/7] arm/arm64: vgic-new: Implement support for userspace access
From: Marc Zyngier @ 2016-09-23 9:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CALicx6uuL9xX3VxLCy+EZi0AqXrNM_O+F3MN+W4gbiriUyb7Ug@mail.gmail.com>
On 22/09/16 15:01, Vijay Kilari wrote:
> On Thu, Sep 22, 2016 at 5:38 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> On 20/09/16 07:12, vijay.kilari at gmail.com wrote:
>>> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>>> +static int vgic_uaccess_read(struct kvm_vcpu *vcpu, struct kvm_io_device *dev,
>>> + gpa_t addr, u32 *val)
>>> +{
>>> + struct vgic_io_device *iodev = kvm_to_vgic_iodev(dev);
>>> + const struct vgic_register_region *region;
>>> + struct kvm_vcpu *r_vcpu;
>>> +
>>> + region = vgic_get_mmio_region(iodev, addr, sizeof(u32));
>>> + if (!region) {
>>> + *val = 0;
>>> + return 0;
>>
>> This is not the previous semantic of vgic_uaccess, and I cannot see why
>> blindly ignoring an access to an undefined region would be acceptable.
>> What am I missing?
>
> AFAIK, the vgic_uaccess is not making any check on undefined region/register.
> However, dispatch_mmio_read/write are returning 0 if check of region is failed
Hmmm. Fair enough. I don't really like it, but that's something for
another day.
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* [PATCH v2.1] arm64: kgdb: handle read-only text / modules
From: Catalin Marinas @ 2016-09-23 9:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160923074208.9899-1-takahiro.akashi@linaro.org>
On Fri, Sep 23, 2016 at 04:42:08PM +0900, AKASHI Takahiro wrote:
> Handle read-only cases (CONFIG_DEBUG_RODATA/CONFIG_DEBUG_SET_MODULE_RONX)
> by using aarch64_insn_write() instead of probe_kernel_write().
> See how this works:
> commit 2f896d586610 ("arm64: use fixmap for text patching")
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> Reviewed-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Jason Wessel <jason.wessel@windriver.com>
> Cc: <stable@vger.kernel.org> # 3.18-3.19: 2f896d5: arm64: use fixmap
> Cc: <stable@vger.kernel.org> # 3.18-3.19: f6242ca: arm64: Fix text
> Cc: <stable@vger.kernel.org> # 4.0-
Queued for 4.8 with a slight change in the last Cc: tag above:
Cc: <stable@vger.kernel.org> # 3.18.x-
Thanks.
--
Catalin
^ permalink raw reply
* [PATCH v3] arm: dts: zynq: Add MicroZed board support
From: Jagan Teki @ 2016-09-23 9:48 UTC (permalink / raw)
To: linux-arm-kernel
From: Jagan Teki <jteki@openedev.com>
Added basic dts support for MicroZed board.
- UART
- SDHCI
- Ethernet
Cc: Soren Brinkmann <soren.brinkmann@xilinx.com>
Cc: Michal Simek <michal.simek@xilinx.com>
Signed-off-by: Jagan Teki <jteki@openedev.com>
---
Changes for v3:
- Add Xilinx copyright
Changes for v2:
- Add SDHCI
- Add Ethernet
arch/arm/boot/dts/Makefile | 1 +
arch/arm/boot/dts/zynq-microzed.dts | 96 +++++++++++++++++++++++++++++++++++++
2 files changed, 97 insertions(+)
create mode 100644 arch/arm/boot/dts/zynq-microzed.dts
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index faacd52..4d7b858 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -862,6 +862,7 @@ dtb-$(CONFIG_ARCH_VT8500) += \
wm8750-apc8750.dtb \
wm8850-w70v2.dtb
dtb-$(CONFIG_ARCH_ZYNQ) += \
+ zynq-microzed.dtb \
zynq-parallella.dtb \
zynq-zc702.dtb \
zynq-zc706.dtb \
diff --git a/arch/arm/boot/dts/zynq-microzed.dts b/arch/arm/boot/dts/zynq-microzed.dts
new file mode 100644
index 0000000..b9376a4
--- /dev/null
+++ b/arch/arm/boot/dts/zynq-microzed.dts
@@ -0,0 +1,96 @@
+/*
+ * Copyright (C) 2011 - 2014 Xilinx
+ * Copyright (C) 2016 Jagan Teki <jteki@openedev.com>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+/dts-v1/;
+/include/ "zynq-7000.dtsi"
+
+/ {
+ model = "Zynq MicroZED Development Board";
+ compatible = "xlnx,zynq-microzed", "xlnx,zynq-7000";
+
+ aliases {
+ ethernet0 = &gem0;
+ serial0 = &uart1;
+ };
+
+ memory {
+ device_type = "memory";
+ reg = <0x0 0x40000000>;
+ };
+
+ chosen {
+ bootargs = "earlycon";
+ stdout-path = "serial0:115200n8";
+ };
+
+ usb_phy0: phy0 {
+ compatible = "usb-nop-xceiv";
+ #phy-cells = <0>;
+ };
+};
+
+&clkc {
+ ps-clk-frequency = <33333333>;
+};
+
+&gem0 {
+ status = "okay";
+ phy-mode = "rgmii-id";
+ phy-handle = <ðernet_phy>;
+
+ ethernet_phy: ethernet-phy at 0 {
+ reg = <0>;
+ };
+};
+
+&sdhci0 {
+ status = "okay";
+};
+
+&uart1 {
+ status = "okay";
+};
+
+&usb0 {
+ status = "okay";
+ dr_mode = "host";
+ usb-phy = <&usb_phy0>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_usb0_default>;
+};
+
+&pinctrl0 {
+ pinctrl_usb0_default: usb0-default {
+ mux {
+ groups = "usb0_0_grp";
+ function = "usb0";
+ };
+
+ conf {
+ groups = "usb0_0_grp";
+ slew-rate = <0>;
+ io-standard = <1>;
+ };
+
+ conf-rx {
+ pins = "MIO29", "MIO31", "MIO36";
+ bias-high-impedance;
+ };
+
+ conf-tx {
+ pins = "MIO28", "MIO30", "MIO32", "MIO33", "MIO34",
+ "MIO35", "MIO37", "MIO38", "MIO39";
+ bias-disable;
+ };
+ };
+};
--
2.7.4
^ permalink raw reply related
* [PATCH 00/24] ste_dma40: Fine-tuning for several function implementations
From: Linus Walleij @ 2016-09-23 9:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <92810066-69b6-94e7-dcec-a28594b1328f@users.sourceforge.net>
On Sat, Sep 17, 2016 at 5:05 PM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 17 Sep 2016 16:56:56 +0200
>
> Several update suggestions were taken into account
> from static source code analysis.
All look very nice!
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Vinod, please apply them all!
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH] arm/arm64: KVM: Add support for ARMv8 AArch32 execution state
From: Marc Zyngier @ 2016-09-23 9:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160923092550.GG9101@cbox>
On 23/09/16 10:25, Christoffer Dall wrote:
> On Fri, Sep 23, 2016 at 03:13:25PM +0800, Alison Wang wrote:
>> The ARMv8 architecture supports two execution state, AArch64 and
>> AArch32. To support KVM in AArch32 execution state for ARMv8, Cortex-A53
>> and Cortex-A72 need to be added for target-specific checks.
>>
>> Signed-off-by: Alison Wang <alison.wang@nxp.com>
>> ---
>> arch/arm/include/asm/cputype.h | 2 ++
>> arch/arm/kvm/guest.c | 2 ++
>> 2 files changed, 4 insertions(+)
>>
>> diff --git a/arch/arm/include/asm/cputype.h b/arch/arm/include/asm/cputype.h
>> index 754f86f..4f8c632 100644
>> --- a/arch/arm/include/asm/cputype.h
>> +++ b/arch/arm/include/asm/cputype.h
>> @@ -75,6 +75,8 @@
>> #define ARM_CPU_PART_CORTEX_A12 0x4100c0d0
>> #define ARM_CPU_PART_CORTEX_A17 0x4100c0e0
>> #define ARM_CPU_PART_CORTEX_A15 0x4100c0f0
>> +#define ARM_CPU_PART_CORTEX_A53_AARCH32 0x4100d030
>> +#define ARM_CPU_PART_CORTEX_A72_AARCH32 0x4100d080
>> #define ARM_CPU_PART_MASK 0xff00fff0
>>
>> /* DEC implemented cores */
>> diff --git a/arch/arm/kvm/guest.c b/arch/arm/kvm/guest.c
>> index 9aca920..462a099 100644
>> --- a/arch/arm/kvm/guest.c
>> +++ b/arch/arm/kvm/guest.c
>> @@ -252,6 +252,8 @@ int __attribute_const__ kvm_target_cpu(void)
>> {
>> switch (read_cpuid_part()) {
>> case ARM_CPU_PART_CORTEX_A7:
>> + case ARM_CPU_PART_CORTEX_A53_AARCH32:
>> + case ARM_CPU_PART_CORTEX_A72_AARCH32:
>
> huh? why are we mapping A53 and A72 cores to an A7 core?
Yeah, that's absolutely disgusting. The only sensible thing to do would
be to implement something "generic", just like we have on the 64bit side.
As it stands, this patch falls into the "No way" bucket.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* [PATCH v14 2/4] CMDQ: Mediatek CMDQ driver
From: Horng-Shyang Liao @ 2016-09-23 9:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CABb+yY1U8WeSWTaeiPJDO1WCBm2AH2QGAiyBextga-KFHwspig@mail.gmail.com>
Hi Jassi,
Please see my inline reply.
On Thu, 2016-09-22 at 13:47 +0530, Jassi Brar wrote:
> On Mon, Sep 5, 2016 at 7:14 AM, HS Liao <hs.liao@mediatek.com> wrote:
[...]
> > +struct cmdq_base *cmdq_register_device(struct device *dev)
> > +{
> > + struct cmdq_base *cmdq_base;
> > + struct resource res;
> > + int subsys;
> > + u32 base;
> > +
> > + if (of_address_to_resource(dev->of_node, 0, &res))
> > + return NULL;
> > + base = (u32)res.start;
> > +
> > + subsys = cmdq_subsys_base_to_id(base >> 16);
> > + if (subsys < 0)
> > + return NULL;
> > +
> > + cmdq_base = devm_kmalloc(dev, sizeof(*cmdq_base), GFP_KERNEL);
> > + if (!cmdq_base)
> > + return NULL;
> > + cmdq_base->subsys = subsys;
> > + cmdq_base->base = base;
> > +
> > + return cmdq_base;
> > +}
> > +EXPORT_SYMBOL(cmdq_register_device);
> > +
> > +struct cmdq_client *cmdq_mbox_create(struct device *dev, int index)
> > +{
> > + struct cmdq_client *client;
> > +
> > + client = kzalloc(sizeof(*client), GFP_KERNEL);
> > + client->client.dev = dev;
> > + client->client.tx_block = false;
> > + client->chan = mbox_request_channel(&client->client, index);
> > + return client;
> > +}
> > +EXPORT_SYMBOL(cmdq_mbox_create);
> > +
> > +int cmdq_task_create(struct device *dev, struct cmdq_task **task_ptr)
> > +{
> > + struct cmdq_task *task;
> > + int err;
> > +
> > + task = kzalloc(sizeof(*task), GFP_KERNEL);
> > + if (!task)
> > + return -ENOMEM;
> > + task->cmdq = dev_get_drvdata(dev);
> > + err = cmdq_task_realloc_cmd_buffer(task, PAGE_SIZE);
> > + if (err < 0) {
> > + kfree(task);
> > + return err;
> > + }
> > + *task_ptr = task;
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(cmdq_task_create);
> > +
> > +static int cmdq_task_append_command(struct cmdq_task *task, enum cmdq_code code,
> > + u32 arg_a, u32 arg_b)
> > +{
> > + u64 *cmd_ptr;
> > + int err;
> > +
> > + if (WARN_ON(task->finalized))
> > + return -EBUSY;
> > + if (unlikely(task->cmd_buf_size + CMDQ_INST_SIZE > task->buf_size)) {
> > + err = cmdq_task_realloc_cmd_buffer(task, task->buf_size * 2);
> > + if (err < 0)
> > + return err;
> > + }
> > + cmd_ptr = task->va_base + task->cmd_buf_size;
> > + (*cmd_ptr) = (u64)((code << CMDQ_OP_CODE_SHIFT) | arg_a) << 32 | arg_b;
> > + task->cmd_buf_size += CMDQ_INST_SIZE;
> > + return 0;
> > +}
> > +
> > +int cmdq_task_write(struct cmdq_task *task, u32 value, struct cmdq_base *base,
> > + u32 offset)
> > +{
> > + u32 arg_a = ((base->base + offset) & CMDQ_ARG_A_WRITE_MASK) |
> > + (base->subsys << CMDQ_SUBSYS_SHIFT);
> > + return cmdq_task_append_command(task, CMDQ_CODE_WRITE, arg_a, value);
> > +}
> > +EXPORT_SYMBOL(cmdq_task_write);
> > +
> > +int cmdq_task_write_mask(struct cmdq_task *task, u32 value,
> > + struct cmdq_base *base, u32 offset, u32 mask)
> > +{
> > + u32 offset_mask = offset;
> > + int err;
> > +
> > + if (mask != 0xffffffff) {
> > + err = cmdq_task_append_command(task, CMDQ_CODE_MASK, 0, ~mask);
> > + if (err < 0)
> > + return err;
> > + offset_mask |= CMDQ_WRITE_ENABLE_MASK;
> > + }
> > + return cmdq_task_write(task, value, base, offset_mask);
> > +}
> > +EXPORT_SYMBOL(cmdq_task_write_mask);
> > +
> > +static const u32 cmdq_event_value[CMDQ_MAX_EVENT] = {
> > + /* Display start of frame(SOF) events */
> > + [CMDQ_EVENT_DISP_OVL0_SOF] = 11,
> > + [CMDQ_EVENT_DISP_OVL1_SOF] = 12,
> > + [CMDQ_EVENT_DISP_RDMA0_SOF] = 13,
> > + [CMDQ_EVENT_DISP_RDMA1_SOF] = 14,
> > + [CMDQ_EVENT_DISP_RDMA2_SOF] = 15,
> > + [CMDQ_EVENT_DISP_WDMA0_SOF] = 16,
> > + [CMDQ_EVENT_DISP_WDMA1_SOF] = 17,
> > + /* Display end of frame(EOF) events */
> > + [CMDQ_EVENT_DISP_OVL0_EOF] = 39,
> > + [CMDQ_EVENT_DISP_OVL1_EOF] = 40,
> > + [CMDQ_EVENT_DISP_RDMA0_EOF] = 41,
> > + [CMDQ_EVENT_DISP_RDMA1_EOF] = 42,
> > + [CMDQ_EVENT_DISP_RDMA2_EOF] = 43,
> > + [CMDQ_EVENT_DISP_WDMA0_EOF] = 44,
> > + [CMDQ_EVENT_DISP_WDMA1_EOF] = 45,
> > + /* Mutex end of frame(EOF) events */
> > + [CMDQ_EVENT_MUTEX0_STREAM_EOF] = 53,
> > + [CMDQ_EVENT_MUTEX1_STREAM_EOF] = 54,
> > + [CMDQ_EVENT_MUTEX2_STREAM_EOF] = 55,
> > + [CMDQ_EVENT_MUTEX3_STREAM_EOF] = 56,
> > + [CMDQ_EVENT_MUTEX4_STREAM_EOF] = 57,
> > + /* Display underrun events */
> > + [CMDQ_EVENT_DISP_RDMA0_UNDERRUN] = 63,
> > + [CMDQ_EVENT_DISP_RDMA1_UNDERRUN] = 64,
> > + [CMDQ_EVENT_DISP_RDMA2_UNDERRUN] = 65,
> > +};
> > +
> > +int cmdq_task_wfe(struct cmdq_task *task, enum cmdq_event event)
> > +{
> > + u32 arg_b;
> > +
> > + if (event >= CMDQ_MAX_EVENT || event < 0)
> > + return -EINVAL;
> > +
> > + /*
> > + * WFE arg_b
> > + * bit 0-11: wait value
> > + * bit 15: 1 - wait, 0 - no wait
> > + * bit 16-27: update value
> > + * bit 31: 1 - update, 0 - no update
> > + */
> > + arg_b = CMDQ_WFE_UPDATE | CMDQ_WFE_WAIT | CMDQ_WFE_WAIT_VALUE;
> > + return cmdq_task_append_command(task, CMDQ_CODE_WFE,
> > + cmdq_event_value[event], arg_b);
> > +}
> > +EXPORT_SYMBOL(cmdq_task_wfe);
> > +
> > +int cmdq_task_clear_event(struct cmdq_task *task, enum cmdq_event event)
> > +{
> > + if (event >= CMDQ_MAX_EVENT || event < 0)
> > + return -EINVAL;
> > +
> > + return cmdq_task_append_command(task, CMDQ_CODE_WFE,
> > + cmdq_event_value[event], CMDQ_WFE_UPDATE);
> > +}
> > +EXPORT_SYMBOL(cmdq_task_clear_event);
> > +
> > +static int cmdq_task_finalize(struct cmdq_task *task)
> > +{
> > + int err;
> > +
> > + if (task->finalized)
> > + return 0;
> > +
> > + /* insert EOC and generate IRQ for each command iteration */
> > + err = cmdq_task_append_command(task, CMDQ_CODE_EOC, 0, CMDQ_EOC_IRQ_EN);
> > + if (err < 0)
> > + return err;
> > +
> > + /* JUMP to end */
> > + err = cmdq_task_append_command(task, CMDQ_CODE_JUMP, 0, CMDQ_JUMP_PASS);
> > + if (err < 0)
> > + return err;
> > +
> > + task->finalized = true;
> > + return 0;
> > +}
> > +
> > +int cmdq_task_flush_async(struct cmdq_client *client, struct cmdq_task *task,
> > + cmdq_async_flush_cb cb, void *data)
> > +{
> > + struct cmdq *cmdq = task->cmdq;
> > + int err;
> > +
> > + mutex_lock(&cmdq->task_mutex);
> > + if (cmdq->suspended) {
> > + dev_err(cmdq->mbox.dev, "%s is called after suspended\n",
> > + __func__);
> > + mutex_unlock(&cmdq->task_mutex);
> > + return -EPERM;
> > + }
> > +
> > + err = cmdq_task_finalize(task);
> > + if (err < 0) {
> > + mutex_unlock(&cmdq->task_mutex);
> > + return err;
> > + }
> > +
> > + INIT_LIST_HEAD(&task->list_entry);
> > + task->cb.cb = cb;
> > + task->cb.data = data;
> > + task->pa_base = dma_map_single(cmdq->mbox.dev, task->va_base,
> > + task->cmd_buf_size, DMA_TO_DEVICE);
> > +
> > + mbox_send_message(client->chan, task);
> > + /* We can send next task immediately, so just call txdone. */
> > + mbox_client_txdone(client->chan, 0);
> > + mutex_unlock(&cmdq->task_mutex);
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(cmdq_task_flush_async);
> > +
> > +struct cmdq_flush_completion {
> > + struct completion cmplt;
> > + bool err;
> > +};
> > +
> > +static void cmdq_task_flush_cb(struct cmdq_cb_data data)
> > +{
> > + struct cmdq_flush_completion *cmplt = data.data;
> > +
> > + cmplt->err = data.err;
> > + complete(&cmplt->cmplt);
> > +}
> > +
> > +int cmdq_task_flush(struct cmdq_client *client, struct cmdq_task *task)
> > +{
> > + struct cmdq_flush_completion cmplt;
> > + int err;
> > +
> > + init_completion(&cmplt.cmplt);
> > + err = cmdq_task_flush_async(client, task, cmdq_task_flush_cb, &cmplt);
> > + if (err < 0)
> > + return err;
> > + wait_for_completion(&cmplt.cmplt);
> > + return cmplt.err ? -EFAULT : 0;
> > +}
> > +EXPORT_SYMBOL(cmdq_task_flush);
> > +
> > +void cmdq_mbox_free(struct cmdq_client *client)
> > +{
> > + mbox_free_channel(client->chan);
> > + kfree(client);
> > +}
> > +EXPORT_SYMBOL(cmdq_mbox_free);
> > +
> All these exported functions implement the protocol, so should not be
> a part of this controller driver. That should go into
> drivers/soc/mediatek/
>
> The controller driver (mtk-cmdq.c) should implement mainly the
> mbox_chan_ops and mbox.of_xlate.
>
I can do that, but I would like to confirm with Matthias in advance.
[...]
> > + cmdq->irq = irq_of_parse_and_map(node, 0);
> >
> why not, cmdq->irq = platform_get_irq(pdev, 0);
Will do
[...]
> > +static struct platform_driver cmdq_drv = {
> > + .probe = cmdq_probe,
> > + .remove = cmdq_remove,
> > + .driver = {
> > + .name = "mtk_cmdq",
> > + .owner = THIS_MODULE,
> >
> please remove the unnecessary .owner field.
Will do
> > + .pm = &cmdq_pm_ops,
> > + .of_match_table = cmdq_of_ids,
> > + }
> > +};
> > +
> > +builtin_platform_driver(cmdq_drv);
> > diff --git a/include/linux/mailbox/mtk-cmdq.h b/include/linux/mailbox/mtk-cmdq.h
> > new file mode 100644
> > index 0000000..c3c924d
> > --- /dev/null
> > +++ b/include/linux/mailbox/mtk-cmdq.h
> >
> The api implemented is Mediateck proprietary, so I think it should be
> include/linux/soc/mediatek/cmdq.h
>
>
> > @@ -0,0 +1,180 @@
> > +/*
> > + * Copyright (c) 2015 MediaTek Inc.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 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 General Public License for more details.
> > + */
> > +
> > +#ifndef __MTK_CMDQ_H__
> > +#define __MTK_CMDQ_H__
> > +
> > +#include <linux/mailbox_client.h>
> > +#include <linux/mailbox_controller.h>
> >
> Clients should not need to include mailbox_controller.h
This is because client needs to know controller's dev.
Please see my CMDQ v13.
http://www.spinics.net/lists/kernel/msg2327867.html
I add mailbox_controller.h for client to get controller's dev,
so I can remove a node reference in device tree.
Should I revert the modification of CMDQ v13?
> > +#include <linux/platform_device.h>
> > +#include <linux/types.h>
> > +
> > +/* display events in command queue(CMDQ) */
> > +enum cmdq_event {
> > + /* Display start of frame(SOF) events */
> > + CMDQ_EVENT_DISP_OVL0_SOF,
> >
> you may want to explicitly initialise the first element.
Will do
> > + CMDQ_EVENT_DISP_OVL1_SOF,
> > + CMDQ_EVENT_DISP_RDMA0_SOF,
> > + CMDQ_EVENT_DISP_RDMA1_SOF,
> > + CMDQ_EVENT_DISP_RDMA2_SOF,
> > + CMDQ_EVENT_DISP_WDMA0_SOF,
> > + CMDQ_EVENT_DISP_WDMA1_SOF,
> > + /* Display end of frame(EOF) events */
> > + CMDQ_EVENT_DISP_OVL0_EOF,
> > + CMDQ_EVENT_DISP_OVL1_EOF,
> > + CMDQ_EVENT_DISP_RDMA0_EOF,
> > + CMDQ_EVENT_DISP_RDMA1_EOF,
> > + CMDQ_EVENT_DISP_RDMA2_EOF,
> > + CMDQ_EVENT_DISP_WDMA0_EOF,
> > + CMDQ_EVENT_DISP_WDMA1_EOF,
> > + /* Mutex end of frame(EOF) events */
> > + CMDQ_EVENT_MUTEX0_STREAM_EOF,
> > + CMDQ_EVENT_MUTEX1_STREAM_EOF,
> > + CMDQ_EVENT_MUTEX2_STREAM_EOF,
> > + CMDQ_EVENT_MUTEX3_STREAM_EOF,
> > + CMDQ_EVENT_MUTEX4_STREAM_EOF,
> > + /* Display underrun events */
> > + CMDQ_EVENT_DISP_RDMA0_UNDERRUN,
> > + CMDQ_EVENT_DISP_RDMA1_UNDERRUN,
> > + CMDQ_EVENT_DISP_RDMA2_UNDERRUN,
> > + /* Keep this at the end */
> > + CMDQ_MAX_EVENT,
> > +};
> > +
Thanks,
HS
Hi Matthias,
Do you agree with Jassi's comments about moving parts of code back to
soc/mediatek/ ?
If I do it, the part in soc/mediatek/ will be similar to a library.
Could you tell me a good way to handle this situation?
Thanks,
HS
^ permalink raw reply
* [PATCH v14 4/4] CMDQ: save more energy in idle
From: Horng-Shyang Liao @ 2016-09-23 9:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CABb+yY1YqyReK5bii6n5a_rJ6fgpPi=aC3sfKJdomvM5+NG9FA@mail.gmail.com>
On Thu, 2016-09-22 at 13:22 +0530, Jassi Brar wrote:
> On Mon, Sep 5, 2016 at 7:14 AM, HS Liao <hs.liao@mediatek.com> wrote:
> > Use clk_disable_unprepare instead of clk_disable to save more energy
> > when CMDQ is idle.
> >
> > Signed-off-by: HS Liao <hs.liao@mediatek.com>
> > ---
> > drivers/mailbox/mtk-cmdq.c | 54 +++++++++++++++++++++++++++++++++++++++-------
>
> The driver is introduced by second patch of the set, so it makes sense
> to merge this patch into patch 2/4.
Hi Jassi,
Could you take a look at previous discussion between Matthias and me?
http://lkml.iu.edu/hypermail/linux/kernel/1606.2/05239.html
His basic idea is to simplify first working version.
Therefore, I move some code to this patch.
Thanks,
HS
^ permalink raw reply
* [PATCH v2 0/3] arm64: kgdb: fix single stepping
From: AKASHI Takahiro @ 2016-09-23 9:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160923084341.GA32342@kroah.com>
On Fri, Sep 23, 2016 at 10:43:41AM +0200, Greg KH wrote:
> On Fri, Sep 23, 2016 at 05:32:58PM +0900, AKASHI Takahiro wrote:
> > On Fri, Sep 23, 2016 at 10:16:18AM +0200, Greg KH wrote:
> > > On Fri, Sep 23, 2016 at 04:33:24PM +0900, AKASHI Takahiro wrote:
> > > > Kgdb support on arm64 was merged in v3.15, but from its first appearance,
> > > > "signle step" has never worked well.
> > > >
> > > > This patch fixes all the error cases I found so far.
> > > > The original patch[1] was splitted into three pieces, ones for each case.
> > > > patch#1, #2 should be applied to all the version, v3.15 and later.
> > > > pathc#3 only for v3.16 and later.
> > >
> > > As this is not a regression (i.e. it has never worked), why is this
> > > something for stable releases?
> >
> > Because, I think, that is a bug.
> > The author seems to have believed that it worked.
> > Please see:
> > commit 44679a4f
> > Author: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
> > Date: Tue Jan 28 11:20:19 2014 +0000
> >
> > arm64: KGDB: Add step debugging support
>
> Yes, but again, it didn't work, so this would be a new feature.
Yes, but again, it's a bug of kgdb on arm64 which is supposed
to have been available since v3.15.
> One
> that obviously people aren't using in the stable kernels, otherwise they
> would have noticed in the past 2 years about it being broken :)
Totally agree. That is also why I've left this patch untouched
for a long time. But recently a guy asked me about kgdb, saying that
single step didn't work on his platform. He re-discovered this bug.
He doesn't use stable kernels, but Linaro's LSK (v4.4), though :)
> Please read Documentation/stable_kernel_rules.txt for the requirements
> of a stable kernel patch. I don't think this series meets those rules.
Please re-think about merging this patch.
Thanks,
-Takahiro AKASHI
> thanks,
>
> greg k-h
^ permalink raw reply
* [PATCH] arm/arm64: KVM: Add support for ARMv8 AArch32 execution state
From: Christoffer Dall @ 2016-09-23 9:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1474614805-18629-1-git-send-email-b18965@freescale.com>
On Fri, Sep 23, 2016 at 03:13:25PM +0800, Alison Wang wrote:
> The ARMv8 architecture supports two execution state, AArch64 and
> AArch32. To support KVM in AArch32 execution state for ARMv8, Cortex-A53
> and Cortex-A72 need to be added for target-specific checks.
>
> Signed-off-by: Alison Wang <alison.wang@nxp.com>
> ---
> arch/arm/include/asm/cputype.h | 2 ++
> arch/arm/kvm/guest.c | 2 ++
> 2 files changed, 4 insertions(+)
>
> diff --git a/arch/arm/include/asm/cputype.h b/arch/arm/include/asm/cputype.h
> index 754f86f..4f8c632 100644
> --- a/arch/arm/include/asm/cputype.h
> +++ b/arch/arm/include/asm/cputype.h
> @@ -75,6 +75,8 @@
> #define ARM_CPU_PART_CORTEX_A12 0x4100c0d0
> #define ARM_CPU_PART_CORTEX_A17 0x4100c0e0
> #define ARM_CPU_PART_CORTEX_A15 0x4100c0f0
> +#define ARM_CPU_PART_CORTEX_A53_AARCH32 0x4100d030
> +#define ARM_CPU_PART_CORTEX_A72_AARCH32 0x4100d080
> #define ARM_CPU_PART_MASK 0xff00fff0
>
> /* DEC implemented cores */
> diff --git a/arch/arm/kvm/guest.c b/arch/arm/kvm/guest.c
> index 9aca920..462a099 100644
> --- a/arch/arm/kvm/guest.c
> +++ b/arch/arm/kvm/guest.c
> @@ -252,6 +252,8 @@ int __attribute_const__ kvm_target_cpu(void)
> {
> switch (read_cpuid_part()) {
> case ARM_CPU_PART_CORTEX_A7:
> + case ARM_CPU_PART_CORTEX_A53_AARCH32:
> + case ARM_CPU_PART_CORTEX_A72_AARCH32:
huh? why are we mapping A53 and A72 cores to an A7 core?
-Christoffer
^ permalink raw reply
* [PATCH v2 0/3] arm64: kgdb: fix single stepping
From: Greg KH @ 2016-09-23 9:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160923092706.GN30248@linaro.org>
On Fri, Sep 23, 2016 at 06:27:07PM +0900, AKASHI Takahiro wrote:
> On Fri, Sep 23, 2016 at 10:43:41AM +0200, Greg KH wrote:
> > On Fri, Sep 23, 2016 at 05:32:58PM +0900, AKASHI Takahiro wrote:
> > > On Fri, Sep 23, 2016 at 10:16:18AM +0200, Greg KH wrote:
> > > > On Fri, Sep 23, 2016 at 04:33:24PM +0900, AKASHI Takahiro wrote:
> > > > > Kgdb support on arm64 was merged in v3.15, but from its first appearance,
> > > > > "signle step" has never worked well.
> > > > >
> > > > > This patch fixes all the error cases I found so far.
> > > > > The original patch[1] was splitted into three pieces, ones for each case.
> > > > > patch#1, #2 should be applied to all the version, v3.15 and later.
> > > > > pathc#3 only for v3.16 and later.
> > > >
> > > > As this is not a regression (i.e. it has never worked), why is this
> > > > something for stable releases?
> > >
> > > Because, I think, that is a bug.
> > > The author seems to have believed that it worked.
> > > Please see:
> > > commit 44679a4f
> > > Author: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
> > > Date: Tue Jan 28 11:20:19 2014 +0000
> > >
> > > arm64: KGDB: Add step debugging support
> >
> > Yes, but again, it didn't work, so this would be a new feature.
>
> Yes, but again, it's a bug of kgdb on arm64 which is supposed
> to have been available since v3.15.
But it wasn't ever working, so it's not a regression or a bugfix, sorry.
> > Please read Documentation/stable_kernel_rules.txt for the requirements
> > of a stable kernel patch. I don't think this series meets those rules.
>
> Please re-think about merging this patch.
It's not even in Linus's tree yet, so there's nothing for me to merge
anywhere.
thanks,
greg k-h
^ permalink raw reply
* [PATCH 3/3] arm64: Add the Raspberry Pi firmware's interface to the FXL6408.
From: Linus Walleij @ 2016-09-23 9:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1474577091.12009.10.camel@redhat.com>
On Thu, Sep 22, 2016 at 10:44 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> On Mo, 2016-09-19 at 17:13 +0100, Eric Anholt wrote:
>> This gets us hotplug detection of HDMI, so that graphics now works at
>> boot. Tested with watching the output of xrandr while plugging and
>> unplugging the HDMI cable.
>
> Very nice. Whole patch series:
>
> Tested-by: Gerd Hoffmann <kraxel@redhat.com>
>
> /me hopes this lands in the 4.9 merge window.
Sorry no way. I have comments on the driver and
bindings.
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH v5] KVM: arm/arm64: Route vtimer events to user space
From: Christoffer Dall @ 2016-09-23 9:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <7afbc08e-0ba2-d6fd-c1ed-e5822e05c00b@suse.de>
On Fri, Sep 23, 2016 at 11:10:46AM +0200, Alexander Graf wrote:
>
>
> On 23.09.16 10:57, Paolo Bonzini wrote:
> >
> >
> > On 23/09/2016 09:14, Alexander Graf wrote:
> >>>> +/*
> >>>> + * Synchronize the timer IRQ state with the interrupt controller.
> >>>> + */
> >>>> static void kvm_timer_update_irq(struct kvm_vcpu *vcpu, bool new_level)
> >>>> {
> >>>> int ret;
> >>>> struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
> >>>>
> >>>> timer->active_cleared_last = false;
> >>>> timer->irq.level = new_level;
> >>>> - trace_kvm_timer_update_irq(vcpu->vcpu_id, timer->irq.irq,
> >>>> + trace_kvm_timer_update_irq(vcpu->vcpu_id, host_vtimer_irq,
> >>>> timer->irq.level);
> >>>> + [...]
> >>>> + struct kvm_sync_regs *regs = &vcpu->run->s.regs;
> >>>> +
> >>>> + /* Populate the timer bitmap for user space */
> >>>> + regs->kernel_timer_pending &= ~KVM_ARM_TIMER_VTIMER;
> >>>> + if (new_level)
> >>>> + regs->kernel_timer_pending |= KVM_ARM_TIMER_VTIMER;
> >>>
> >>> I think if you got here, it means you have to exit to userspace to
> >>> update it of the new state. If you don't want to propagate a return
> >>
> >> Yes, but we can't exit straight away with our own exit reason because we
> >> might be inside an MMIO exit path here which already occupies the
> >> exit_reason.
> >
> > So the idea is that whenever you're here you have one of the following
> > cases:
> >
> > - are coming from kvm_timer_flush_hwstate, and then you exit immediately
> > with KVM_EXIT_INTR if needed
> >
> > - you are coming from the kvm_timer_sync_hwstate just before
> > handle_exit. Then if there's a vmexit you have already set
> > regs->kernel_timer_pending, if not you'll do a kvm_timer_flush_hwstate soon.
> >
> > - you are coming from the kvm_timer_sync_hwstate in the middle of
> > kvm_arch_vcpu_ioctl_run, and then "continue" will either exit the loop
> > immediately (if ret <= 0) or go to kvm_timer_flush_hwstate as in the
> > previous case
> >
> > Right?
>
> Yup :)
>
> >
> >>> Maybe I'm misunderstanding and user_timer_pending is just a cached
> >>> verison of what you said last, but as I said above, I think you can just
> >>> compare timer->irq.level with the last value the kvm_run struct, and if
> >>> something changed, you have to exit.
> >>
> >> So how would user space know whether the line went up or down? Or didn't
> >> change at all (if we coalesce with an MMIO exit)?
> >
> > It would save the status of the line somewhere in its own variable,
> > without introducing a relatively delicate API between kernel and user.
> >
> > I agree that you cannot update kernel_timer_pending only in
> > flush_hwstate; otherwise you miss on case (2) when there is a vmexit.
> > That has to stay in kvm_timer_sync_hwstate (or it could become a new
> > function called at the very end of kvm_arch_vcpu_ioctl_run).
>
> The beauty of having it in the timer update function is that it gets
> called from flush_hwstate() as well. That way we catch the update also
> in cases where we need it before we enter the vcpu.
>
> > However, I'm not sure why user_timer_pending is necessary. If it is
> > just the value you assigned to regs->kernel_timer_pending at the time of
> > the previous vmexit, can kvm_timer_flush_hwstate just do
> >
> > if (timer->prev_kernel_timer_pending != regs->kernel_timer_pending) {
> > timer->prev_kernel_timer_pending = regs->kernel_timer_pending;
> > return 1;
> > }
> >
> > ? Or even
> >
> > if (timer->prev_irq_level != timer->irq.level) {
> > timer->prev_irq_level = regs->irq.level;
> > return 1;
> > }
> >
> > so that regs->kernel_timer_pending remains exclusively
> > kvm_timer_sync_hwstate's business?
>
> We could do that too, yes. But it puts more burden on user space - it
> would have to ensure that it *always* checks for the pending timer
> status. With this approach, user space may opt to only check for timer
> state changes on -EINTR and things would still work.
>
Huh? I thought the whole point was that you could piggy back on an MMIO
exit with a change in the line state, for example. So I don't
understand this.
In any case, exposing internal historical state only used by the kernel
to figure out whether or not it should force an exit if there's not
already one happening, to user space, just feels weird to me, and my
emphasis is on having a clean timer emulation component in the kernel
where the semantics are clear. Have a look at my proposal in the other
mail on this thread.
-Christoffer
^ permalink raw reply
* [PATCH] gpio: Added zynq specific check for special pins on bank zero
From: Linus Walleij @ 2016-09-23 9:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1474360324-8168-1-git-send-email-navam@xilinx.com>
On Tue, Sep 20, 2016 at 10:32 AM, Nava kishore Manne
<nava.manne@xilinx.com> wrote:
> From: Nava kishore Manne <nava.manne@xilinx.com>
>
> This patch adds zynq specific check for bank 0 pins 7 and 8
> are special and cannot be used as inputs
>
> Signed-off-by: Nava kishore Manne <navam@xilinx.com>
I'm waiting for a revised version of this patch with comments adressed
and ACKs added.
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH v5] KVM: arm/arm64: Route vtimer events to user space
From: Paolo Bonzini @ 2016-09-23 9:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <70fc788d-f99e-4ba5-67a2-da9c1a857dfe@suse.de>
On 23/09/2016 11:17, Alexander Graf wrote:
>> >
>> > On the other hand, what happens if you run new QEMU with old userspace?
>> > With user_timer_pending you'd get an infinite stream of vmexits the
>> > first time the timer fires, wouldn't you? Whereas if you keep it in the
>> > kernel, userspace would simply not get the interrupt (because it doesn't
>> > know about kernel_timer_pending) and think it got a spurious vmexit.
>> > The kernel's IRQ would stay masked and everything would just (not) work
>> > like before your patch?
> Yes, we'd definitely stay more compatible by tracking it only in the
> kernel. I'm not fully convinced that it's the better interface, but
> since both Christoffer and you seem to choke on that part, I'll give it
> a stab ;).
Cool! FWIW my suggestion for kernel_timer_pending's name would be
timer_irq_level (nicely matching timer->irq.level in the kernel).
Paolo
^ permalink raw reply
* [RFC PATCH 0/3] efi: MMC proxy support for the UEFI varstore
From: Mark Rutland @ 2016-09-23 9:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKv+Gu-H=JkXFSm2bO4tFcUEVPs2ueT3za2X+aUjrnGZZSdUvQ@mail.gmail.com>
On Thu, Sep 22, 2016 at 02:37:00PM +0100, Ard Biesheuvel wrote:
> On 22 September 2016 at 13:58, Mark Rutland <mark.rutland@arm.com> wrote:
> > On Thu, Sep 22, 2016 at 12:30:03PM +0100, Ard Biesheuvel wrote:
> > I also think that this needs to go via the USWG (and to the UEFI spec),
> > before we can consider using it.
> The primary issue there is that the variable protocols are
> architectural DXE protocols defined in the PI spec, not the UEFI spec
> (and the MMC host protocol I am using here is not in any spec afaik),
> and so implementing this in a UEFI compliant way is not currently
> possible. Exposing a UEFI protocol like block I/O is not sufficient,
> since the variable protocols are depended upon earlier in the boot
> sequence (in UEFI implementations based on PI).
>
> So yes, this needs to be discussed (and it will be, but not out in the
> open, unfortunately.) However, it is unclear whether it is feasible to
> address this at the UEFI level, or whether we will need to go beyond
> and define something based on the PI spec directly. (/me looks at
> Charles)
That is... less than optimal. :(
> > I have a few other general concerns:
> > * Lifetime guarantees
> >
> > When is it valid for EFI to call the MMC proxy? Can other services
> > (e.g. ACPI) call this?
> >
> > How do we handle kexec/kdump? e.g. how do we teardown the interface
> > before branching to a new kernel, how do we safely tear down a crashed
> > kernel's interface, what can we call before doing so?
> >
> > How do we handle suspend/resume? e.g. is it necessary to re-register
> > upon resume?
>
> All good questions, and exactly the kind of feedback I am seeking.
>
> Tearing down the interface could be as simple as clearing the pointer,
> but some synchronization is probably in order to make sure that no
> calls are in progress.
>
> But to clarify the purpose of this series: are there any concerns
> regarding exposing callbacks to the firmware in general, and for MMC
> access in particular, from the Linux side?
I have a general uneasiness about having UEFI call kernel function
pointers, inverting the usual relationship, but I'll need to think about
that a little further -- the lifetime stuff was the most obvious problem
in that area, but I'm sure there are more. ;)
It would really help to know *when* UEFI is expected/allowed to call
this. For synchronous calls, the FW could return a new
requires-os-support error code, and the OS could then call some API,
figure out what to do, and later send any acknowledgement required back
to UEFI. That might not cover all cases, though.
Thanks,
Mark.
^ permalink raw reply
* [PATCH v5] KVM: arm/arm64: Route vtimer events to user space
From: Alexander Graf @ 2016-09-23 9:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <e4c08096-03bd-3ec3-998d-41643992ab1a@redhat.com>
On 23.09.16 11:15, Paolo Bonzini wrote:
>
>
> On 23/09/2016 11:10, Alexander Graf wrote:
>>>>> Maybe I'm misunderstanding and user_timer_pending is just a cached
>>>>> verison of what you said last, but as I said above, I think you can just
>>>>> compare timer->irq.level with the last value the kvm_run struct, and if
>>>>> something changed, you have to exit.
>>>>
>>>> So how would user space know whether the line went up or down? Or didn't
>>>> change at all (if we coalesce with an MMIO exit)?
>>>
>>> It would save the status of the line somewhere in its own variable,
>>> without introducing a relatively delicate API between kernel and user.
>>>
>>> I agree that you cannot update kernel_timer_pending only in
>>> flush_hwstate; otherwise you miss on case (2) when there is a vmexit.
>>> That has to stay in kvm_timer_sync_hwstate (or it could become a new
>>> function called at the very end of kvm_arch_vcpu_ioctl_run).
>>
>> The beauty of having it in the timer update function is that it gets
>> called from flush_hwstate() as well. That way we catch the update also
>> in cases where we need it before we enter the vcpu.
>
> Yeah, the timer update function is ideal because it is called from both
> flush_hwstate and sync_hwstate.
>
>>> However, I'm not sure why user_timer_pending is necessary. If it is
>>> just the value you assigned to regs->kernel_timer_pending at the time of
>>> the previous vmexit, can kvm_timer_flush_hwstate just do
>>>
>>> if (timer->prev_kernel_timer_pending != regs->kernel_timer_pending) {
>>> timer->prev_kernel_timer_pending = regs->kernel_timer_pending;
>>> return 1;
>>> }
>>>
>>> ? Or even
>>>
>>> if (timer->prev_irq_level != timer->irq.level) {
>>> timer->prev_irq_level = regs->irq.level;
>>> return 1;
>>> }
>>>
>>> so that regs->kernel_timer_pending remains exclusively
>>> kvm_timer_sync_hwstate's business?
>>
>> We could do that too, yes. But it puts more burden on user space - it
>> would have to ensure that it *always* checks for the pending timer
>> status. With this approach, user space may opt to only check for timer
>> state changes on -EINTR and things would still work.
>
> That would be overloading EINTR a bit though. But that's a digression,
> I don't think it matters much.
>
> On the other hand, what happens if you run new QEMU with old userspace?
> With user_timer_pending you'd get an infinite stream of vmexits the
> first time the timer fires, wouldn't you? Whereas if you keep it in the
> kernel, userspace would simply not get the interrupt (because it doesn't
> know about kernel_timer_pending) and think it got a spurious vmexit.
> The kernel's IRQ would stay masked and everything would just (not) work
> like before your patch?
Yes, we'd definitely stay more compatible by tracking it only in the
kernel. I'm not fully convinced that it's the better interface, but
since both Christoffer and you seem to choke on that part, I'll give it
a stab ;).
Alex
^ permalink raw reply
* [PATCH v5] KVM: arm/arm64: Route vtimer events to user space
From: Paolo Bonzini @ 2016-09-23 9:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <7afbc08e-0ba2-d6fd-c1ed-e5822e05c00b@suse.de>
On 23/09/2016 11:10, Alexander Graf wrote:
>>>> Maybe I'm misunderstanding and user_timer_pending is just a cached
>>>> verison of what you said last, but as I said above, I think you can just
>>>> compare timer->irq.level with the last value the kvm_run struct, and if
>>>> something changed, you have to exit.
>>>
>>> So how would user space know whether the line went up or down? Or didn't
>>> change at all (if we coalesce with an MMIO exit)?
>>
>> It would save the status of the line somewhere in its own variable,
>> without introducing a relatively delicate API between kernel and user.
>>
>> I agree that you cannot update kernel_timer_pending only in
>> flush_hwstate; otherwise you miss on case (2) when there is a vmexit.
>> That has to stay in kvm_timer_sync_hwstate (or it could become a new
>> function called at the very end of kvm_arch_vcpu_ioctl_run).
>
> The beauty of having it in the timer update function is that it gets
> called from flush_hwstate() as well. That way we catch the update also
> in cases where we need it before we enter the vcpu.
Yeah, the timer update function is ideal because it is called from both
flush_hwstate and sync_hwstate.
>> However, I'm not sure why user_timer_pending is necessary. If it is
>> just the value you assigned to regs->kernel_timer_pending at the time of
>> the previous vmexit, can kvm_timer_flush_hwstate just do
>>
>> if (timer->prev_kernel_timer_pending != regs->kernel_timer_pending) {
>> timer->prev_kernel_timer_pending = regs->kernel_timer_pending;
>> return 1;
>> }
>>
>> ? Or even
>>
>> if (timer->prev_irq_level != timer->irq.level) {
>> timer->prev_irq_level = regs->irq.level;
>> return 1;
>> }
>>
>> so that regs->kernel_timer_pending remains exclusively
>> kvm_timer_sync_hwstate's business?
>
> We could do that too, yes. But it puts more burden on user space - it
> would have to ensure that it *always* checks for the pending timer
> status. With this approach, user space may opt to only check for timer
> state changes on -EINTR and things would still work.
That would be overloading EINTR a bit though. But that's a digression,
I don't think it matters much.
On the other hand, what happens if you run new QEMU with old userspace?
With user_timer_pending you'd get an infinite stream of vmexits the
first time the timer fires, wouldn't you? Whereas if you keep it in the
kernel, userspace would simply not get the interrupt (because it doesn't
know about kernel_timer_pending) and think it got a spurious vmexit.
The kernel's IRQ would stay masked and everything would just (not) work
like before your patch?
Thanks,
Paolo
^ permalink raw reply
* [PATCH v2 2/2] net: thunderx: Support for byte queue limits
From: sunil.kovvuri at gmail.com @ 2016-09-23 9:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1474621948-9172-1-git-send-email-sunil.kovvuri@gmail.com>
From: Sunil Goutham <sgoutham@cavium.com>
This patch adds support for byte queue limits
Signed-off-by: Sunil Goutham <sgoutham@cavium.com>
---
drivers/net/ethernet/cavium/thunder/nicvf_main.c | 14 ++++++++--
drivers/net/ethernet/cavium/thunder/nicvf_queues.c | 30 ++++++++++++++--------
2 files changed, 32 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
index 7d00162..45a13f7 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
@@ -516,7 +516,8 @@ static int nicvf_init_resources(struct nicvf *nic)
static void nicvf_snd_pkt_handler(struct net_device *netdev,
struct cmp_queue *cq,
struct cqe_send_t *cqe_tx,
- int cqe_type, int budget)
+ int cqe_type, int budget,
+ unsigned int *tx_pkts, unsigned int *tx_bytes)
{
struct sk_buff *skb = NULL;
struct nicvf *nic = netdev_priv(netdev);
@@ -547,6 +548,8 @@ static void nicvf_snd_pkt_handler(struct net_device *netdev,
}
nicvf_put_sq_desc(sq, hdr->subdesc_cnt + 1);
prefetch(skb);
+ (*tx_pkts)++;
+ *tx_bytes += skb->len;
napi_consume_skb(skb, budget);
sq->skbuff[cqe_tx->sqe_ptr] = (u64)NULL;
} else {
@@ -662,6 +665,7 @@ static int nicvf_cq_intr_handler(struct net_device *netdev, u8 cq_idx,
struct cmp_queue *cq = &qs->cq[cq_idx];
struct cqe_rx_t *cq_desc;
struct netdev_queue *txq;
+ unsigned int tx_pkts = 0, tx_bytes = 0;
spin_lock_bh(&cq->lock);
loop:
@@ -701,7 +705,7 @@ loop:
case CQE_TYPE_SEND:
nicvf_snd_pkt_handler(netdev, cq,
(void *)cq_desc, CQE_TYPE_SEND,
- budget);
+ budget, &tx_pkts, &tx_bytes);
tx_done++;
break;
case CQE_TYPE_INVALID:
@@ -730,6 +734,9 @@ done:
netdev = nic->pnicvf->netdev;
txq = netdev_get_tx_queue(netdev,
nicvf_netdev_qidx(nic, cq_idx));
+ if (tx_pkts)
+ netdev_tx_completed_queue(txq, tx_pkts, tx_bytes);
+
nic = nic->pnicvf;
if (netif_tx_queue_stopped(txq) && netif_carrier_ok(netdev)) {
netif_tx_start_queue(txq);
@@ -1160,6 +1167,9 @@ int nicvf_stop(struct net_device *netdev)
netif_tx_disable(netdev);
+ for (qidx = 0; qidx < netdev->num_tx_queues; qidx++)
+ netdev_tx_reset_queue(netdev_get_tx_queue(netdev, qidx));
+
/* Free resources */
nicvf_config_data_transfer(nic, false);
diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_queues.c b/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
index 178c5c7..a4fc501 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
@@ -1082,6 +1082,24 @@ static inline void nicvf_sq_add_cqe_subdesc(struct snd_queue *sq, int qentry,
imm->len = 1;
}
+static inline void nicvf_sq_doorbell(struct nicvf *nic, struct sk_buff *skb,
+ int sq_num, int desc_cnt)
+{
+ struct netdev_queue *txq;
+
+ txq = netdev_get_tx_queue(nic->pnicvf->netdev,
+ skb_get_queue_mapping(skb));
+
+ netdev_tx_sent_queue(txq, skb->len);
+
+ /* make sure all memory stores are done before ringing doorbell */
+ smp_wmb();
+
+ /* Inform HW to xmit all TSO segments */
+ nicvf_queue_reg_write(nic, NIC_QSET_SQ_0_7_DOOR,
+ sq_num, desc_cnt);
+}
+
/* Segment a TSO packet into 'gso_size' segments and append
* them to SQ for transfer
*/
@@ -1141,12 +1159,8 @@ static int nicvf_sq_append_tso(struct nicvf *nic, struct snd_queue *sq,
/* Save SKB in the last segment for freeing */
sq->skbuff[hdr_qentry] = (u64)skb;
- /* make sure all memory stores are done before ringing doorbell */
- smp_wmb();
+ nicvf_sq_doorbell(nic, skb, sq_num, desc_cnt);
- /* Inform HW to xmit all TSO segments */
- nicvf_queue_reg_write(nic, NIC_QSET_SQ_0_7_DOOR,
- sq_num, desc_cnt);
nic->drv_stats.tx_tso++;
return 1;
}
@@ -1219,12 +1233,8 @@ doorbell:
nicvf_sq_add_cqe_subdesc(sq, qentry, tso_sqe, skb);
}
- /* make sure all memory stores are done before ringing doorbell */
- smp_wmb();
+ nicvf_sq_doorbell(nic, skb, sq_num, subdesc_cnt);
- /* Inform HW to xmit new packet */
- nicvf_queue_reg_write(nic, NIC_QSET_SQ_0_7_DOOR,
- sq_num, subdesc_cnt);
return 1;
append_fail:
--
2.7.4
^ permalink raw reply related
* [PATCH v2 1/2] net: thunderx: Fix issue with IRQ namimg
From: sunil.kovvuri at gmail.com @ 2016-09-23 9:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1474621948-9172-1-git-send-email-sunil.kovvuri@gmail.com>
From: Sunil Goutham <sgoutham@cavium.com>
This patch fixes a regression caused by previous commit
when irq name exceeds 20 byte array if interface's name
size is large.
Fixes: e412621394fa ("net: thunderx: Use netdev's name for naming VF's interrupts")
Signed-off-by: Sunil Goutham <sgoutham@cavium.com>
---
drivers/net/ethernet/cavium/thunder/nic.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/cavium/thunder/nic.h b/drivers/net/ethernet/cavium/thunder/nic.h
index 18d12d3..3042610 100644
--- a/drivers/net/ethernet/cavium/thunder/nic.h
+++ b/drivers/net/ethernet/cavium/thunder/nic.h
@@ -305,7 +305,7 @@ struct nicvf {
bool msix_enabled;
u8 num_vec;
struct msix_entry msix_entries[NIC_VF_MSIX_VECTORS];
- char irq_name[NIC_VF_MSIX_VECTORS][20];
+ char irq_name[NIC_VF_MSIX_VECTORS][IFNAMSIZ + 15];
bool irq_allocated[NIC_VF_MSIX_VECTORS];
cpumask_var_t affinity_mask[NIC_VF_MSIX_VECTORS];
--
2.7.4
^ permalink raw reply related
* [PATCH v2 0/2] BQL support and fix for a regression issue
From: sunil.kovvuri at gmail.com @ 2016-09-23 9:12 UTC (permalink / raw)
To: linux-arm-kernel
From: Sunil Goutham <sgoutham@cavium.com>
These patches add byte queue limit support and also fixes a regression
issue introduced by commit
'net: thunderx: Use netdev's name for naming VF's interrupts'
Changes from v1:
- As suggested added 'Fixes' tag with commit id of previous commit
which cuased issue.
- Also fixed the missing netdev_tx_reset_queue() function call in
byte queue limits support patch.
Sunil Goutham (2):
net: thunderx: Fix issue with IRQ namimg
net: thunderx: Support for byte queue limits
drivers/net/ethernet/cavium/thunder/nic.h | 2 +-
drivers/net/ethernet/cavium/thunder/nicvf_main.c | 14 ++++++++--
drivers/net/ethernet/cavium/thunder/nicvf_queues.c | 30 ++++++++++++++--------
3 files changed, 33 insertions(+), 13 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH v5] KVM: arm/arm64: Route vtimer events to user space
From: Alexander Graf @ 2016-09-23 9:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <63a1e3a2-f551-017f-b6be-593b53a0b407@redhat.com>
On 23.09.16 10:57, Paolo Bonzini wrote:
>
>
> On 23/09/2016 09:14, Alexander Graf wrote:
>>>> +/*
>>>> + * Synchronize the timer IRQ state with the interrupt controller.
>>>> + */
>>>> static void kvm_timer_update_irq(struct kvm_vcpu *vcpu, bool new_level)
>>>> {
>>>> int ret;
>>>> struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
>>>>
>>>> timer->active_cleared_last = false;
>>>> timer->irq.level = new_level;
>>>> - trace_kvm_timer_update_irq(vcpu->vcpu_id, timer->irq.irq,
>>>> + trace_kvm_timer_update_irq(vcpu->vcpu_id, host_vtimer_irq,
>>>> timer->irq.level);
>>>> + [...]
>>>> + struct kvm_sync_regs *regs = &vcpu->run->s.regs;
>>>> +
>>>> + /* Populate the timer bitmap for user space */
>>>> + regs->kernel_timer_pending &= ~KVM_ARM_TIMER_VTIMER;
>>>> + if (new_level)
>>>> + regs->kernel_timer_pending |= KVM_ARM_TIMER_VTIMER;
>>>
>>> I think if you got here, it means you have to exit to userspace to
>>> update it of the new state. If you don't want to propagate a return
>>
>> Yes, but we can't exit straight away with our own exit reason because we
>> might be inside an MMIO exit path here which already occupies the
>> exit_reason.
>
> So the idea is that whenever you're here you have one of the following
> cases:
>
> - are coming from kvm_timer_flush_hwstate, and then you exit immediately
> with KVM_EXIT_INTR if needed
>
> - you are coming from the kvm_timer_sync_hwstate just before
> handle_exit. Then if there's a vmexit you have already set
> regs->kernel_timer_pending, if not you'll do a kvm_timer_flush_hwstate soon.
>
> - you are coming from the kvm_timer_sync_hwstate in the middle of
> kvm_arch_vcpu_ioctl_run, and then "continue" will either exit the loop
> immediately (if ret <= 0) or go to kvm_timer_flush_hwstate as in the
> previous case
>
> Right?
Yup :)
>
>>> Maybe I'm misunderstanding and user_timer_pending is just a cached
>>> verison of what you said last, but as I said above, I think you can just
>>> compare timer->irq.level with the last value the kvm_run struct, and if
>>> something changed, you have to exit.
>>
>> So how would user space know whether the line went up or down? Or didn't
>> change at all (if we coalesce with an MMIO exit)?
>
> It would save the status of the line somewhere in its own variable,
> without introducing a relatively delicate API between kernel and user.
>
> I agree that you cannot update kernel_timer_pending only in
> flush_hwstate; otherwise you miss on case (2) when there is a vmexit.
> That has to stay in kvm_timer_sync_hwstate (or it could become a new
> function called at the very end of kvm_arch_vcpu_ioctl_run).
The beauty of having it in the timer update function is that it gets
called from flush_hwstate() as well. That way we catch the update also
in cases where we need it before we enter the vcpu.
> However, I'm not sure why user_timer_pending is necessary. If it is
> just the value you assigned to regs->kernel_timer_pending at the time of
> the previous vmexit, can kvm_timer_flush_hwstate just do
>
> if (timer->prev_kernel_timer_pending != regs->kernel_timer_pending) {
> timer->prev_kernel_timer_pending = regs->kernel_timer_pending;
> return 1;
> }
>
> ? Or even
>
> if (timer->prev_irq_level != timer->irq.level) {
> timer->prev_irq_level = regs->irq.level;
> return 1;
> }
>
> so that regs->kernel_timer_pending remains exclusively
> kvm_timer_sync_hwstate's business?
We could do that too, yes. But it puts more burden on user space - it
would have to ensure that it *always* checks for the pending timer
status. With this approach, user space may opt to only check for timer
state changes on -EINTR and things would still work.
Alex
^ 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