Re: [RFC PATCH 0/6] Add platform device SVM support for ARM SMMUv3

2017-09-06 Thread Bob Liu
On 2017/9/6 17:59, Jean-Philippe Brucker wrote:
> On 06/09/17 02:16, Yisheng Xie wrote:
>> Hi Jean-Philippe,
>>
>> On 2017/9/5 20:56, Jean-Philippe Brucker wrote:
>>> On 31/08/17 09:20, Yisheng Xie wrote:
 Jean-Philippe has post a patchset for Adding PCIe SVM support to ARM 
 SMMUv3:
 https://www.spinics.net/lists/arm-kernel/msg565155.html

 But for some platform devices(aka on-chip integrated devices), there is 
 also
 SVM requirement, which works based on the SMMU stall mode.
 Jean-Philippe has prepared a prototype patchset to support it:
 git://linux-arm.org/linux-jpb.git svm/stall
>>>
>>> Only meant for testing at that point, and unfit even for an RFC.
>>
>> Sorry about that, I should ask you before send it out. It's my mistake. For 
>> I also
>> have some question about this patchset.
>>
>> We have related device, and would like to do some help about it. Do you have
>> any plan about upstream ?
>>
>>>
 We tested this patchset with some fixes on a on-chip integrated device. The
 basic function is ok, so I just send them out for review, although this
 patchset heavily depends on the former patchset (PCIe SVM support for ARM
 SMMUv3), which is still under discussion.

 Patch Overview:
 *1 to 3 prepare for device tree or acpi get the device stall ability and 
 pasid bits
 *4 is to realise the SVM function for platform device
 *5 is fix a bug when test SVM function while SMMU donnot support this 
 feature
 *6 avoid ILLEGAL setting of STE and CD entry about stall

 Acctually here, I also have some questions about SVM on SMMUv3:

 1. Why the SVM feature on SMMUv3 depends on BTM feature? when bind a task 
 to device,
it will register a mmu_notify. Therefore, when a page range is invalid, 
 we can
send TLBI or ATC invalid without BTM?
>>>
>>> We could, but the end goal for SVM is to perfectly mirror the CPU page
>>> tables. So for platform SVM we would like to get rid of MMU notifiers
>>> entirely.
>>
>> I see, but for some SMMU which do not support BTM, it cannot benefit from 
>> SVM.
>>
>> Meanwhile, do you mean even with BTM feature, the PCI-e device also need to 
>> send a
>> ATC invalid by MMU notify? It seems not fair, why not hardware do the 
>> entirely work
>> in this case? It may costly for send ATC invalid and sync.
> 
> It will certainly be costly. But there are major problems with
> transforming broadcast TLB maintenance into ATC invalidations in HW:
> 
> * VMID:ASID to SID:SSID conversion. TLBIs use VMID:ASID, while ATCIs use
> SID:SSID.
> 
> * Most importantly, ATC invalidations accounting. Each endpoint has a
> limited number of in-flight ATC invalidate requests. The conversion module
> would have to buffer incoming invalidations and wait for in-flight ATC
> invalidation to complete before sending the next ones. In case of
> overflow, either we lose invalidation (which opens security holes) or we
> somehow put back-pressure on the interconnect (no idea how feasible this
> is, I suspect really hard).
> 
> Solving the last one is also quite difficult in software, but at least we
> can still invalidate a range. In hardware we would invalidate the ATC
> page-by-page and quickly jam the bus.
> 

Speak to the invalidation, I have one more question.

There is a time window between 1) modify page table;  2) tlb invalidate;

ARM-CPU   Device

1. modify page table

 ^
  Can still write data through smmu tlb even page 
table was already modified.
  (At this point, the same virtual addr may not 
point to the same thing for CPU and device!!!
   I'm afraid there may be some data-loss or other 
potential problems if this situation happens.)

2. tlb invalidate range

--
Thanks,
Bob

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [RFC PATCH 0/6] Add platform device SVM support for ARM SMMUv3

2017-09-06 Thread Bob Liu
On 2017/9/6 17:57, Jean-Philippe Brucker wrote:
> On 06/09/17 02:02, Bob Liu wrote:
>> On 2017/9/5 20:56, Jean-Philippe Brucker wrote:
>>> On 31/08/17 09:20, Yisheng Xie wrote:
 Jean-Philippe has post a patchset for Adding PCIe SVM support to ARM 
 SMMUv3:
 https://www.spinics.net/lists/arm-kernel/msg565155.html

 But for some platform devices(aka on-chip integrated devices), there is 
 also
 SVM requirement, which works based on the SMMU stall mode.
 Jean-Philippe has prepared a prototype patchset to support it:
 git://linux-arm.org/linux-jpb.git svm/stall
>>>
>>> Only meant for testing at that point, and unfit even for an RFC.
>>>
>>
>> Sorry for the misunderstanding.
>> The PRI mode patches is in RFC even no hardware for testing, so I thought 
>> it's fine for "Stall mode" patches sent as RFC.
>> We have tested the Stall mode on our platform.
>> Anyway, I should confirm with you in advance.
>>
>> Btw, Would you consider the "stall mode" upstream at first? Since there is 
>> no hardware for testing the PRI mode.
>> (We can provide you the hardware which support SMMU stall mode if necessary.)
> 
> Yes. What's blocking the ATS, PRI and PASID patches at the moment is the
> lack of endpoints for testing. There has been lots of discussion on the
> API side since my first RFC and I'd like to resubmit the API changes soon.
> It is the same API for ATS+PRI+PASID and SSID+Stall, so the backend
> doesn't matter.
> 

Indeed!

> I'm considering upstreaming SSID+Stall first if it can be tested on
> hardware (having direct access to it would certainly speed things up).

Glad to hear that.

> This would require some work in moving the PCI bits at the end of the
> series. I can reserve some time in the coming months to do it, but I need
> to know what to focus on. Are you able to test SSID as well?
> 

Yes, but the difficulty is our devices are on-chip integrated hardware 
accelerators which requires complicate driver.
You may need much time to understand the driver.
That's the same case as intel/amd SVM, the current user is their GPU :-(

Btw, what kind of device/method do you think is ideal for testing arm-SVM?

 We tested this patchset with some fixes on a on-chip integrated device. The
 basic function is ok, so I just send them out for review, although this
 patchset heavily depends on the former patchset (PCIe SVM support for ARM
 SMMUv3), which is still under discussion.

 Patch Overview:
 *1 to 3 prepare for device tree or acpi get the device stall ability and 
 pasid bits
 *4 is to realise the SVM function for platform device
 *5 is fix a bug when test SVM function while SMMU donnot support this 
 feature
 *6 avoid ILLEGAL setting of STE and CD entry about stall

 Acctually here, I also have a question about SVM on SMMUv3:

 1. Why the SVM feature on SMMUv3 depends on BTM feature? when bind a task 
 to device,
it will register a mmu_notify. Therefore, when a page range is invalid, 
 we can
send TLBI or ATC invalid without BTM?
>>>
>>> We could, but the end goal for SVM is to perfectly mirror the CPU page
>>> tables. So for platform SVM we would like to get rid of MMU notifiers
>>> entirely.
>>>
 2. According to ACPI IORT spec, named component specific data has a node 
 flags field
whoes bit0 is for Stall support. However, it do not have any field for 
 pasid bit.
Can we use other 5 bits[5:1] for pasid bit numbers, so we can have 32 
 pasid bit for
a single platform device which should be enough, because SMMU only 
 support 20 bit pasid

>>
>> Any comment on this?
>> The ACPI IORT spec may need be updated?
> 
> I suppose that the Named Component Node could be used for SSID and stall
> capability bits. Can't the ACPI namespace entries be extended to host
> these capabilities in a more generic way? Platforms with different IOMMUs
> might also need this information some day.
> 

Hmm, that would be better.
But in anyway, it depends on the ACPI IORT Spec would be extended in next 
version.

--
Thanks,
Bob Liu



___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH] drivers/iommu/tegra: Optimize mutex_unlock function call

2017-09-06 Thread Pushkar Jambhlekar
Code can be optimized more by making single exit point of function and calling 
mutex_unlock at the end

Signed-off-by: Pushkar Jambhlekar 
---
 drivers/iommu/tegra-smmu.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 3b6449e..da38833 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -232,20 +232,22 @@ static inline void smmu_flush(struct tegra_smmu *smmu)
 static int tegra_smmu_alloc_asid(struct tegra_smmu *smmu, unsigned int *idp)
 {
unsigned long id;
+   int ret = 0;
 
mutex_lock(&smmu->lock);
 
id = find_first_zero_bit(smmu->asids, smmu->soc->num_asids);
if (id >= smmu->soc->num_asids) {
-   mutex_unlock(&smmu->lock);
-   return -ENOSPC;
+   ret = -ENOSPC;
+   goto Exit;
}
 
set_bit(id, smmu->asids);
*idp = id;
 
+Exit:
mutex_unlock(&smmu->lock);
-   return 0;
+   return ret;
 }
 
 static void tegra_smmu_free_asid(struct tegra_smmu *smmu, unsigned int id)
-- 
2.7.4

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [RFC] virtio-iommu version 0.4

2017-09-06 Thread Jean-Philippe Brucker
Hi Kevin,

On 28/08/17 08:39, Tian, Kevin wrote:
> Here comes some comments:
> 
> 1.1 Motivation
> 
> You describe I/O page faults handling as future work. Seems you considered
> only recoverable fault (since "aka. PCI PRI" being used). What about other
> unrecoverable faults e.g. what to do if a virtual DMA request doesn't find 
> a valid mapping? Even when there is no PRI support, we need some basic
> form of fault reporting mechanism to indicate such errors to guest.

I am considering recoverable faults as the end goal, but reporting
unrecoverable faults should use the same queue, with slightly different
fields and no need for the driver to reply to the device.

> 2.6.8.2 Property RESV_MEM
> 
> I'm not immediately clear when VIRTIO_IOMMU_PROBE_RESV_MEM_T_ABORT
> should be explicitly reported. Is there any real example on bare metal IOMMU?
> usually reserved memory is reported to CPU through other method (e.g. e820
> on x86 platform). Of course MSI is a special case which is covered by BYPASS 
> and MSI flag... If yes, maybe you can also include an example in 
> implementation 
> notes.

The RESV_MEM regions only describes IOVA space for the moment, not
guest-physical, so I guess it provides different information than e820.

I think a useful example is the PCI bridge windows reported by the Linux
host to userspace using RESV_RESERVED regions (see
iommu_dma_get_resv_regions). If I understand correctly, they represent DMA
addresses that shouldn't be accessed by endpoints because they won't reach
the IOMMU. These are specific to the physical topology: a device will have
different reserved regions depending on the PCI slot it occupies.

When handled properly, PCI bridge windows quickly become a nuisance. With
kvmtool we observed that carving out their addresses globally removes a
lot of useful GPA space from the guest. Without a virtual IOMMU we can
either ignore them and hope everything will be fine, or remove all
reserved regions from the GPA space (which currently means editing by hand
the static guest-physical map...)

That's where RESV_MEM_T_ABORT comes handy with virtio-iommu. It describes
reserved IOVAs for a specific endpoint, and therefore removes the need to
carve the window out of the whole guest.

> Another thing I want to ask your opinion, about whether there is value of
> adding another subtype (MEM_T_IDENTITY), asking for identity mapping
> in the address space. It's similar to Reserved Memory Region Reporting
> (RMRR) structure defined in VT-d, to indicate BIOS allocated reserved
> memory ranges which may be DMA target and has to be identity mapped
> when DMA remapping is enabled. I'm not sure whether ARM has similar
> capability and whether there might be a general usage beyond VT-d. For
> now the only usage in my mind is to assign a device with RMRR associated
> on VT-d (Intel GPU, or some USB controllers) where the RMRR info needs
> propagated to the guest (since identity mapping also means reservation
> of virtual address space).

Yes I think adding MEM_T_IDENTITY will be necessary. I can see they are
used for both iGPU and USB controllers on my x86 machines. Do you know
more precisely what they are used for by the firmware?

It's not necessary with the base virtio-iommu device though (v0.4),
because the device can create the identity mappings itself and report them
to the guest as MEM_T_BYPASS. However, when we start handing page table
control over to the guest, the host won't be in control of IOVA->GPA
mappings and will need to gracefully ask the guest to do it.

I'm not aware of any firmware description resembling Intel RMRR or AMD
IVMD on ARM platforms. I do think ARM platforms could need MEM_T_IDENTITY
for requesting the guest to map MSI windows when page-table handover is in
use (MSI addresses are translated by the physical SMMU, so a IOVA->GPA
mapping must be installed by the guest). But since a vSMMU would need a
solution as well, I think I'll try to implement something more generic.

> 2.6.8.2.3 Device Requirements: Property RESV_MEM
> 
> --citation start--
> If an endpoint is attached to an address space, the device SHOULD leave 
> any access targeting one of its VIRTIO_IOMMU_PROBE_RESV_MEM_T_BYPASS 
> regions pass through untranslated. In other words, the device SHOULD 
> handle such a region as if it was identity-mapped (virtual address equal to
> physical address). If the endpoint is not attached to any address space, 
> then the device MAY abort the transaction.
> --citation end
> 
> I have a question for the last sentence. From definition of BYPASS, it's
> orthogonal to whether there is an address space attached, then should
> we still allow "May abort" behavior? 

The behavior is left as an implementation choice, and I'm not sure it's
worth enforcing in the architecture. If the endpoint isn't attached to any
domain then (unless VIRTIO_IOMMU_F_BYPASS is negotiated), it isn't
necessarily able to do DMA at all. The virtio-iommu device may setup DMA
mastering lazily, in which 

Re: [virtio-dev] [RFC] virtio-iommu version 0.4

2017-09-06 Thread Jean-Philippe Brucker
Hi Eric,

On 23/08/17 14:55, Auger Eric wrote:
> Please find some comments/questions below:

Thanks a lot for this. Sorry for the delay, I was on holiday and it took
me a while to sort out the details.

> 2.6.7:1
> I do not understand the footnode #6 sentence: 'Without a specific
> definition of the ACK requirements for a given property type, it simply
> means that the driver read all fields of that property."

That doesn't serve any purpose, I'll remove the footnote.

> 2.6.7:2
> what if the driver has not provided a big enough buffer and the device
> cannot report all supported properties?

Good point, how about adding the following to 2.6.7.2 - Device
Requirements: PROBE Request

If the properties array is smaller than \field{probe_size}, then the
device SHOULD NOT write any property and SHOULD set the request
\field{status} to VIRTIO_IOMMU_S_INVAL.

> 2.6.8.2:
> Couldn't we use the same terminology as iommu_resv_type in iommu.h?
> the negative form is not the easiest to understand for me.
> Why F_MSI?

It also needs to be understandable by someone not familiar with the Linux
APIs. It's not entirely obvious to me what the iommu.h regions are without
looking at the implementation, maybe using names from the device point of
view is more clear. I don't mind simplifying though, as I don't see a
reason for the driver to know about BYPASS regions other than
HW MSIs.

How about I remove ABORT and BYPASS, make the only subtype RESV_MEM_T_MSI
(1)? We'd use subtype 0 as "don't care, just don't map this". Can call it
RESV_MEM_T_RESERVED. Flags won't be used for these subtypes.

Another subtype will be RESV_MEM_T_IDENTITY (see my reply to Kevin).
IDENTITY regions must be identity-mapped by the guest and, as I understand
it, are virtual addresses used for DMA by firmware. The flags field will
then contain read/write protection flags.

> 2.6.8.2.1
> Multiple overlapping RESV_MEM properties are merged together. Device
> requirement? if same types I assume?

Combination rules apply to device and driver. When the driver puts
multiple endpoints in the same domain, combination rules apply. They will
become important once the guest attempts to do things like combining
endpoints with different PASID capabilities in the same domain. I'm
considering these endpoints to be behind different physical IOMMUs.

For reserved regions, we specify what the driver should do and what the
device should enforce with regard to mapping IOVAs of overlapping regions.
For example, if a device has some virtual address RESV_T_MSI and an other
device has the same virtual address RESV_T_IDENTITY, what should the
driver do?

I think it should apply the RESV_T_IDENTITY. RESV_T_MSI is just a special
case of RESV_T_RESERVED, it's a hint for the IRQ subsystem and doesn't
have a meaning within a domain. From DMA mappings point of view, it is
effectively the same as RESV_T_RESERVED. When merging RESV_T_RESERVED and
RESV_T_IDENTITY, we should make it RESV_T_IDENTITY. Because it is required
for one endpoint to work (the one with IDENTITY) and is not accessed by
the other (the driver must not allocate this IOVA range for DMA).

More generally, I'd like to add the following combination table to the
spec, that shows the resulting reserved region type after merging regions
from two endpoints. N: no reserved region, R: RESV_T_RESERVED, M:
RESV_T_MSI, I: RESV_T_IDENTITY. It is symmetric so I didn't fill the
bottom half.

  | N | R | M | I
--
N | N | R | M | ?
--
R |   | R | R | I
--
M |   |   | M | I
--
I |   |   |   | I

The 'I' column is problematic. If one endpoint has an IDENTITY region and
the other doesn't have anything at that virtual address, then making that
region an identity mapping will result in the second endpoint being able
to access firmware memory. If using nested translation, stage-2 can help
us here. But in general we should allow the device to reject an attach
that would result in a N+I combination if the host considers it too dodgy.
I think the other combinations are safe enough.

> what if the device is a physical assigned device. does the device report
> the host doorbell or the guest doorbell, may be worth to clarify.

Indeed. For a physical assigned device it is the guest doorbell as it
corresponds to the virtual ITS or APIC (though with the APIC it's the same
region).

> 3.1.1 last paragraph
> you talk about device ID but this is a terminology only known by ARM I
> think. Actually it is introduced later in 3.1.2.

Indeed, I'll move the explanation up.

> 3.1.2
> About ACPI integration. Isn't IORT ARM specific?

IORT is currently edited by ARM, as is virtio-iommu. Their contents are
not ARM specific however, they describe software interfaces that solve
mostly generic problems, and aim to be open.

IORT describes the relation between endpoints and their translation
agents. This problem has already been solved by IVRS, DMAR and IORT. From

[PATCH v8] vfio: platform: reset: Add Broadcom FlexRM reset module

2017-09-06 Thread Anup Patel via iommu
This patch adds Broadcom FlexRM low-level reset for
VFIO platform.

It will do the following:
1. Disable/Deactivate each FlexRM ring
2. Flush each FlexRM ring

The cleanup sequence for FlexRM rings is adapted from
Broadcom FlexRM mailbox driver.

Signed-off-by: Anup Patel 
Reviewed-by: Oza Oza 
Reviewed-by: Scott Branden 
Reviewed-by: Eric Auger 
---
 drivers/vfio/platform/reset/Kconfig|   9 ++
 drivers/vfio/platform/reset/Makefile   |   1 +
 .../vfio/platform/reset/vfio_platform_bcmflexrm.c  | 115 +
 3 files changed, 125 insertions(+)
 create mode 100644 drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c

diff --git a/drivers/vfio/platform/reset/Kconfig 
b/drivers/vfio/platform/reset/Kconfig
index 705..392e3c0 100644
--- a/drivers/vfio/platform/reset/Kconfig
+++ b/drivers/vfio/platform/reset/Kconfig
@@ -13,3 +13,12 @@ config VFIO_PLATFORM_AMDXGBE_RESET
  Enables the VFIO platform driver to handle reset for AMD XGBE
 
  If you don't know what to do here, say N.
+
+config VFIO_PLATFORM_BCMFLEXRM_RESET
+   tristate "VFIO support for Broadcom FlexRM reset"
+   depends on VFIO_PLATFORM && (ARCH_BCM_IPROC || COMPILE_TEST)
+   default ARCH_BCM_IPROC
+   help
+ Enables the VFIO platform driver to handle reset for Broadcom FlexRM
+
+ If you don't know what to do here, say N.
diff --git a/drivers/vfio/platform/reset/Makefile 
b/drivers/vfio/platform/reset/Makefile
index 93f4e23..8d9874b 100644
--- a/drivers/vfio/platform/reset/Makefile
+++ b/drivers/vfio/platform/reset/Makefile
@@ -5,3 +5,4 @@ ccflags-y += -Idrivers/vfio/platform
 
 obj-$(CONFIG_VFIO_PLATFORM_CALXEDAXGMAC_RESET) += vfio-platform-calxedaxgmac.o
 obj-$(CONFIG_VFIO_PLATFORM_AMDXGBE_RESET) += vfio-platform-amdxgbe.o
+obj-$(CONFIG_VFIO_PLATFORM_BCMFLEXRM_RESET) += vfio_platform_bcmflexrm.o
diff --git a/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c 
b/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
new file mode 100644
index 000..5a17d50
--- /dev/null
+++ b/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
@@ -0,0 +1,115 @@
+/*
+ * Copyright (C) 2017 Broadcom
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see .
+ */
+
+/*
+ * This driver provides reset support for Broadcom FlexRM ring manager
+ * to VFIO platform.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "vfio_platform_private.h"
+
+/* FlexRM configuration */
+#define RING_REGS_SIZE 0x1
+#define RING_VER_MAGIC 0x76303031
+
+/* Per-Ring register offsets */
+#define RING_VER   0x000
+#define RING_CONTROL   0x034
+#define RING_FLUSH_DONE0x038
+
+/* Register RING_CONTROL fields */
+#define CONTROL_FLUSH_SHIFT5
+
+/* Register RING_FLUSH_DONE fields */
+#define FLUSH_DONE_MASK0x1
+
+static int vfio_platform_bcmflexrm_shutdown(void __iomem *ring)
+{
+   unsigned int timeout;
+
+   /* Disable/inactivate ring */
+   writel_relaxed(0x0, ring + RING_CONTROL);
+
+   /* Set ring flush state */
+   timeout = 1000; /* timeout of 1s */
+   writel_relaxed(BIT(CONTROL_FLUSH_SHIFT), ring + RING_CONTROL);
+   do {
+   if (readl_relaxed(ring + RING_FLUSH_DONE) &
+   FLUSH_DONE_MASK)
+   break;
+   mdelay(1);
+   } while (--timeout);
+   if (!timeout)
+   return -ETIMEDOUT;
+
+   /* Clear ring flush state */
+   timeout = 1000; /* timeout of 1s */
+   writel_relaxed(0x0, ring + RING_CONTROL);
+   do {
+   if (!(readl_relaxed(ring + RING_FLUSH_DONE) &
+ FLUSH_DONE_MASK))
+   break;
+   mdelay(1);
+   } while (--timeout);
+   if (!timeout)
+   return -ETIMEDOUT;
+
+   return 0;
+}
+
+static int vfio_platform_bcmflexrm_reset(struct vfio_platform_device *vdev)
+{
+   void __iomem *ring;
+   int rc = 0, ret = 0, ring_num = 0;
+   struct vfio_platform_region *reg = &vdev->regions[0];
+
+   /* Map FlexRM ring registers if not mapped */
+   if (!reg->ioaddr) {
+   reg->ioaddr = ioremap_nocache(reg->addr, reg->size);
+   if (!reg->ioaddr)
+  

[PATCH v8] FlexRM support in VFIO platform

2017-09-06 Thread Anup Patel via iommu
This patchset primarily adds Broadcom FlexRM reset module for
VFIO platform driver.

The patches are based on Linux-4.13-rc3 and can also be
found at flexrm-vfio-v8 branch of
https://github.com/Broadcom/arm64-linux.git

Changes since v7:
 - Use "ret |= rc" instead of "ret = rc" in
   vfio_platform_bcmflexrm_reset()

Changes since v6:
 - Update the FlexRM ring flush sequence as suggested
   by HW folks
 - Shutdown all FlexRM ring anyway even if it fails on
   any of the FlexRM rings
 - Use dev_warn() instead of pr_warn()

Changes since v5:
 - Make kconfig option VFIO_PLATFORM_BCMFLEXRM_RESET
   default to ARCH_BCM_IPROC

Changes since v4:
 - Use "--timeout" instead of "timeout--" in
   vfio_platform_bcmflexrm_shutdown()

Changes since v3:
 - Improve "depends on" for Kconfig option
   VFIO_PLATFORM_BCMFLEXRM_RESET
 - Fix typo in pr_warn() called by
   vfio_platform_bcmflexrm_shutdown()
 - Return error from vfio_platform_bcmflexrm_shutdown()
   when FlexRM ring flush timeout happens

Changes since v2:
 - Remove PATCH1 because fixing VFIO no-IOMMU mode is
   a separate topic

Changes since v1:
 - Remove iommu_present() check in vfio_iommu_group_get()
 - Drop PATCH1-to-PATCH3 because IOMMU_CAP_BYPASS is not
   required
 - Move additional comments out of license header in
   vfio_platform_bcmflexrm.c

Anup Patel (1):
  vfio: platform: reset: Add Broadcom FlexRM reset module

 drivers/vfio/platform/reset/Kconfig|   9 ++
 drivers/vfio/platform/reset/Makefile   |   1 +
 .../vfio/platform/reset/vfio_platform_bcmflexrm.c  | 115 +
 3 files changed, 125 insertions(+)
 create mode 100644 drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c

-- 
2.7.4

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v7] vfio: platform: reset: Add Broadcom FlexRM reset module

2017-09-06 Thread Anup Patel via iommu
On Wed, Sep 6, 2017 at 4:39 PM, Auger Eric  wrote:
> Hi Anup,
>
> On 06/09/2017 10:16, Anup Patel wrote:
>> This patch adds Broadcom FlexRM low-level reset for
>> VFIO platform.
>>
>> It will do the following:
>> 1. Disable/Deactivate each FlexRM ring
>> 2. Flush each FlexRM ring
>>
>> The cleanup sequence for FlexRM rings is adapted from
>> Broadcom FlexRM mailbox driver.
>>
>> Signed-off-by: Anup Patel 
>> Reviewed-by: Oza Oza 
>> Reviewed-by: Scott Branden 
>> Reviewed-by: Eric Auger 
>> ---
>>  drivers/vfio/platform/reset/Kconfig|   9 ++
>>  drivers/vfio/platform/reset/Makefile   |   1 +
>>  .../vfio/platform/reset/vfio_platform_bcmflexrm.c  | 115 
>> +
>>  3 files changed, 125 insertions(+)
>>  create mode 100644 drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
>>
>> diff --git a/drivers/vfio/platform/reset/Kconfig 
>> b/drivers/vfio/platform/reset/Kconfig
>> index 705..392e3c0 100644
>> --- a/drivers/vfio/platform/reset/Kconfig
>> +++ b/drivers/vfio/platform/reset/Kconfig
>> @@ -13,3 +13,12 @@ config VFIO_PLATFORM_AMDXGBE_RESET
>> Enables the VFIO platform driver to handle reset for AMD XGBE
>>
>> If you don't know what to do here, say N.
>> +
>> +config VFIO_PLATFORM_BCMFLEXRM_RESET
>> + tristate "VFIO support for Broadcom FlexRM reset"
>> + depends on VFIO_PLATFORM && (ARCH_BCM_IPROC || COMPILE_TEST)
>> + default ARCH_BCM_IPROC
>> + help
>> +   Enables the VFIO platform driver to handle reset for Broadcom FlexRM
>> +
>> +   If you don't know what to do here, say N.
>> diff --git a/drivers/vfio/platform/reset/Makefile 
>> b/drivers/vfio/platform/reset/Makefile
>> index 93f4e23..8d9874b 100644
>> --- a/drivers/vfio/platform/reset/Makefile
>> +++ b/drivers/vfio/platform/reset/Makefile
>> @@ -5,3 +5,4 @@ ccflags-y += -Idrivers/vfio/platform
>>
>>  obj-$(CONFIG_VFIO_PLATFORM_CALXEDAXGMAC_RESET) += 
>> vfio-platform-calxedaxgmac.o
>>  obj-$(CONFIG_VFIO_PLATFORM_AMDXGBE_RESET) += vfio-platform-amdxgbe.o
>> +obj-$(CONFIG_VFIO_PLATFORM_BCMFLEXRM_RESET) += vfio_platform_bcmflexrm.o
>> diff --git a/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c 
>> b/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
>> new file mode 100644
>> index 000..5f89066
>> --- /dev/null
>> +++ b/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
>> @@ -0,0 +1,115 @@
>> +/*
>> + * Copyright (C) 2017 Broadcom
>> + *
>> + * 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.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program.  If not, see .
>> + */
>> +
>> +/*
>> + * This driver provides reset support for Broadcom FlexRM ring manager
>> + * to VFIO platform.
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include "vfio_platform_private.h"
>> +
>> +/* FlexRM configuration */
>> +#define RING_REGS_SIZE   0x1
>> +#define RING_VER_MAGIC   0x76303031
>> +
>> +/* Per-Ring register offsets */
>> +#define RING_VER 0x000
>> +#define RING_CONTROL 0x034
>> +#define RING_FLUSH_DONE  0x038
>> +
>> +/* Register RING_CONTROL fields */
>> +#define CONTROL_FLUSH_SHIFT  5
>> +
>> +/* Register RING_FLUSH_DONE fields */
>> +#define FLUSH_DONE_MASK  0x1
>> +
>> +static int vfio_platform_bcmflexrm_shutdown(void __iomem *ring)
>> +{
>> + unsigned int timeout;
>> +
>> + /* Disable/inactivate ring */
>> + writel_relaxed(0x0, ring + RING_CONTROL);
>> +
>> + /* Set ring flush state */
>> + timeout = 1000; /* timeout of 1s */
>> + writel_relaxed(BIT(CONTROL_FLUSH_SHIFT), ring + RING_CONTROL);
>> + do {
>> + if (readl_relaxed(ring + RING_FLUSH_DONE) &
>> + FLUSH_DONE_MASK)
>> + break;
>> + mdelay(1);
>> + } while (--timeout);
>> + if (!timeout)
>> + return -ETIMEDOUT;
>> +
>> + /* Clear ring flush state */
>> + timeout = 1000; /* timeout of 1s */
>> + writel_relaxed(0x0, ring + RING_CONTROL);
>> + do {
>> + if (!(readl_relaxed(ring + RING_FLUSH_DONE) &
>> +   FLUSH_DONE_MASK))
>> + break;
>> + mdelay(1);
>> + } while (--timeout);
>> + if (!timeout)
>> + return -ETIMEDOUT

Re: [PATCH v7] vfio: platform: reset: Add Broadcom FlexRM reset module

2017-09-06 Thread Auger Eric
Hi Anup,

On 06/09/2017 10:16, Anup Patel wrote:
> This patch adds Broadcom FlexRM low-level reset for
> VFIO platform.
> 
> It will do the following:
> 1. Disable/Deactivate each FlexRM ring
> 2. Flush each FlexRM ring
> 
> The cleanup sequence for FlexRM rings is adapted from
> Broadcom FlexRM mailbox driver.
> 
> Signed-off-by: Anup Patel 
> Reviewed-by: Oza Oza 
> Reviewed-by: Scott Branden 
> Reviewed-by: Eric Auger 
> ---
>  drivers/vfio/platform/reset/Kconfig|   9 ++
>  drivers/vfio/platform/reset/Makefile   |   1 +
>  .../vfio/platform/reset/vfio_platform_bcmflexrm.c  | 115 
> +
>  3 files changed, 125 insertions(+)
>  create mode 100644 drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
> 
> diff --git a/drivers/vfio/platform/reset/Kconfig 
> b/drivers/vfio/platform/reset/Kconfig
> index 705..392e3c0 100644
> --- a/drivers/vfio/platform/reset/Kconfig
> +++ b/drivers/vfio/platform/reset/Kconfig
> @@ -13,3 +13,12 @@ config VFIO_PLATFORM_AMDXGBE_RESET
> Enables the VFIO platform driver to handle reset for AMD XGBE
>  
> If you don't know what to do here, say N.
> +
> +config VFIO_PLATFORM_BCMFLEXRM_RESET
> + tristate "VFIO support for Broadcom FlexRM reset"
> + depends on VFIO_PLATFORM && (ARCH_BCM_IPROC || COMPILE_TEST)
> + default ARCH_BCM_IPROC
> + help
> +   Enables the VFIO platform driver to handle reset for Broadcom FlexRM
> +
> +   If you don't know what to do here, say N.
> diff --git a/drivers/vfio/platform/reset/Makefile 
> b/drivers/vfio/platform/reset/Makefile
> index 93f4e23..8d9874b 100644
> --- a/drivers/vfio/platform/reset/Makefile
> +++ b/drivers/vfio/platform/reset/Makefile
> @@ -5,3 +5,4 @@ ccflags-y += -Idrivers/vfio/platform
>  
>  obj-$(CONFIG_VFIO_PLATFORM_CALXEDAXGMAC_RESET) += 
> vfio-platform-calxedaxgmac.o
>  obj-$(CONFIG_VFIO_PLATFORM_AMDXGBE_RESET) += vfio-platform-amdxgbe.o
> +obj-$(CONFIG_VFIO_PLATFORM_BCMFLEXRM_RESET) += vfio_platform_bcmflexrm.o
> diff --git a/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c 
> b/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
> new file mode 100644
> index 000..5f89066
> --- /dev/null
> +++ b/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
> @@ -0,0 +1,115 @@
> +/*
> + * Copyright (C) 2017 Broadcom
> + *
> + * 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.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see .
> + */
> +
> +/*
> + * This driver provides reset support for Broadcom FlexRM ring manager
> + * to VFIO platform.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "vfio_platform_private.h"
> +
> +/* FlexRM configuration */
> +#define RING_REGS_SIZE   0x1
> +#define RING_VER_MAGIC   0x76303031
> +
> +/* Per-Ring register offsets */
> +#define RING_VER 0x000
> +#define RING_CONTROL 0x034
> +#define RING_FLUSH_DONE  0x038
> +
> +/* Register RING_CONTROL fields */
> +#define CONTROL_FLUSH_SHIFT  5
> +
> +/* Register RING_FLUSH_DONE fields */
> +#define FLUSH_DONE_MASK  0x1
> +
> +static int vfio_platform_bcmflexrm_shutdown(void __iomem *ring)
> +{
> + unsigned int timeout;
> +
> + /* Disable/inactivate ring */
> + writel_relaxed(0x0, ring + RING_CONTROL);
> +
> + /* Set ring flush state */
> + timeout = 1000; /* timeout of 1s */
> + writel_relaxed(BIT(CONTROL_FLUSH_SHIFT), ring + RING_CONTROL);
> + do {
> + if (readl_relaxed(ring + RING_FLUSH_DONE) &
> + FLUSH_DONE_MASK)
> + break;
> + mdelay(1);
> + } while (--timeout);
> + if (!timeout)
> + return -ETIMEDOUT;
> +
> + /* Clear ring flush state */
> + timeout = 1000; /* timeout of 1s */
> + writel_relaxed(0x0, ring + RING_CONTROL);
> + do {
> + if (!(readl_relaxed(ring + RING_FLUSH_DONE) &
> +   FLUSH_DONE_MASK))
> + break;
> + mdelay(1);
> + } while (--timeout);
> + if (!timeout)
> + return -ETIMEDOUT;
> +
> + return 0;
> +}
> +
> +static int vfio_platform_bcmflexrm_reset(struct vfio_platform_device *vdev)
> +{
> + void __iomem *ring;
> + int rc = 0, ret = 0, ring_nu

[PATCH] iommu/amd: Use raw_cpu_ptr() instead of get_cpu_ptr() for ->flush_queue

2017-09-06 Thread Sebastian Andrzej Siewior
get_cpu_ptr() disables preemption and returns the ->flush_queue object
of the current CPU. raw_cpu_ptr() does the same except that it not
disable preemption which means the scheduler can move it to another CPU
after it obtained the per-CPU object.
In this case this is not bad because the data structure itself is
protected with a spin_lock. This change shouldn't matter in general
but on RT it does because the sleeping lock can't be accessed with
disabled preemption.

Cc: Joerg Roedel 
Cc: iommu@lists.linux-foundation.org
Reported-by: vina...@gmail.com
Signed-off-by: Sebastian Andrzej Siewior 
---
 drivers/iommu/amd_iommu.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 4ad7e5e31943..943efbc08128 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -1911,7 +1911,7 @@ static void queue_add(struct dma_ops_domain *dom,
pages = __roundup_pow_of_two(pages);
address >>= PAGE_SHIFT;
 
-   queue = get_cpu_ptr(dom->flush_queue);
+   queue = raw_cpu_ptr(dom->flush_queue);
spin_lock_irqsave(&queue->lock, flags);
 
/*
@@ -1940,8 +1940,6 @@ static void queue_add(struct dma_ops_domain *dom,
 
if (atomic_cmpxchg(&dom->flush_timer_on, 0, 1) == 0)
mod_timer(&dom->flush_timer, jiffies + msecs_to_jiffies(10));
-
-   put_cpu_ptr(dom->flush_queue);
 }
 
 static void queue_flush_timeout(unsigned long data)
-- 
2.14.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [RFC PATCH 0/6] Add platform device SVM support for ARM SMMUv3

2017-09-06 Thread Jean-Philippe Brucker
On 06/09/17 02:16, Yisheng Xie wrote:
> Hi Jean-Philippe,
> 
> On 2017/9/5 20:56, Jean-Philippe Brucker wrote:
>> On 31/08/17 09:20, Yisheng Xie wrote:
>>> Jean-Philippe has post a patchset for Adding PCIe SVM support to ARM SMMUv3:
>>> https://www.spinics.net/lists/arm-kernel/msg565155.html
>>>
>>> But for some platform devices(aka on-chip integrated devices), there is also
>>> SVM requirement, which works based on the SMMU stall mode.
>>> Jean-Philippe has prepared a prototype patchset to support it:
>>> git://linux-arm.org/linux-jpb.git svm/stall
>>
>> Only meant for testing at that point, and unfit even for an RFC.
> 
> Sorry about that, I should ask you before send it out. It's my mistake. For I 
> also
> have some question about this patchset.
> 
> We have related device, and would like to do some help about it. Do you have
> any plan about upstream ?
> 
>>
>>> We tested this patchset with some fixes on a on-chip integrated device. The
>>> basic function is ok, so I just send them out for review, although this
>>> patchset heavily depends on the former patchset (PCIe SVM support for ARM
>>> SMMUv3), which is still under discussion.
>>>
>>> Patch Overview:
>>> *1 to 3 prepare for device tree or acpi get the device stall ability and 
>>> pasid bits
>>> *4 is to realise the SVM function for platform device
>>> *5 is fix a bug when test SVM function while SMMU donnot support this 
>>> feature
>>> *6 avoid ILLEGAL setting of STE and CD entry about stall
>>>
>>> Acctually here, I also have some questions about SVM on SMMUv3:
>>>
>>> 1. Why the SVM feature on SMMUv3 depends on BTM feature? when bind a task 
>>> to device,
>>>it will register a mmu_notify. Therefore, when a page range is invalid, 
>>> we can
>>>send TLBI or ATC invalid without BTM?
>>
>> We could, but the end goal for SVM is to perfectly mirror the CPU page
>> tables. So for platform SVM we would like to get rid of MMU notifiers
>> entirely.
> 
> I see, but for some SMMU which do not support BTM, it cannot benefit from SVM.
> 
> Meanwhile, do you mean even with BTM feature, the PCI-e device also need to 
> send a
> ATC invalid by MMU notify? It seems not fair, why not hardware do the 
> entirely work
> in this case? It may costly for send ATC invalid and sync.

It will certainly be costly. But there are major problems with
transforming broadcast TLB maintenance into ATC invalidations in HW:

* VMID:ASID to SID:SSID conversion. TLBIs use VMID:ASID, while ATCIs use
SID:SSID.

* Most importantly, ATC invalidations accounting. Each endpoint has a
limited number of in-flight ATC invalidate requests. The conversion module
would have to buffer incoming invalidations and wait for in-flight ATC
invalidation to complete before sending the next ones. In case of
overflow, either we lose invalidation (which opens security holes) or we
somehow put back-pressure on the interconnect (no idea how feasible this
is, I suspect really hard).

Solving the last one is also quite difficult in software, but at least we
can still invalidate a range. In hardware we would invalidate the ATC
page-by-page and quickly jam the bus.

Thanks,
Jean
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [RFC PATCH 0/6] Add platform device SVM support for ARM SMMUv3

2017-09-06 Thread Jean-Philippe Brucker
On 06/09/17 02:02, Bob Liu wrote:
> On 2017/9/5 20:56, Jean-Philippe Brucker wrote:
>> On 31/08/17 09:20, Yisheng Xie wrote:
>>> Jean-Philippe has post a patchset for Adding PCIe SVM support to ARM SMMUv3:
>>> https://www.spinics.net/lists/arm-kernel/msg565155.html
>>>
>>> But for some platform devices(aka on-chip integrated devices), there is also
>>> SVM requirement, which works based on the SMMU stall mode.
>>> Jean-Philippe has prepared a prototype patchset to support it:
>>> git://linux-arm.org/linux-jpb.git svm/stall
>>
>> Only meant for testing at that point, and unfit even for an RFC.
>>
> 
> Sorry for the misunderstanding.
> The PRI mode patches is in RFC even no hardware for testing, so I thought 
> it's fine for "Stall mode" patches sent as RFC.
> We have tested the Stall mode on our platform.
> Anyway, I should confirm with you in advance.
> 
> Btw, Would you consider the "stall mode" upstream at first? Since there is no 
> hardware for testing the PRI mode.
> (We can provide you the hardware which support SMMU stall mode if necessary.)

Yes. What's blocking the ATS, PRI and PASID patches at the moment is the
lack of endpoints for testing. There has been lots of discussion on the
API side since my first RFC and I'd like to resubmit the API changes soon.
It is the same API for ATS+PRI+PASID and SSID+Stall, so the backend
doesn't matter.

I'm considering upstreaming SSID+Stall first if it can be tested on
hardware (having direct access to it would certainly speed things up).
This would require some work in moving the PCI bits at the end of the
series. I can reserve some time in the coming months to do it, but I need
to know what to focus on. Are you able to test SSID as well?

>>> We tested this patchset with some fixes on a on-chip integrated device. The
>>> basic function is ok, so I just send them out for review, although this
>>> patchset heavily depends on the former patchset (PCIe SVM support for ARM
>>> SMMUv3), which is still under discussion.
>>>
>>> Patch Overview:
>>> *1 to 3 prepare for device tree or acpi get the device stall ability and 
>>> pasid bits
>>> *4 is to realise the SVM function for platform device
>>> *5 is fix a bug when test SVM function while SMMU donnot support this 
>>> feature
>>> *6 avoid ILLEGAL setting of STE and CD entry about stall
>>>
>>> Acctually here, I also have a question about SVM on SMMUv3:
>>>
>>> 1. Why the SVM feature on SMMUv3 depends on BTM feature? when bind a task 
>>> to device,
>>>it will register a mmu_notify. Therefore, when a page range is invalid, 
>>> we can
>>>send TLBI or ATC invalid without BTM?
>>
>> We could, but the end goal for SVM is to perfectly mirror the CPU page
>> tables. So for platform SVM we would like to get rid of MMU notifiers
>> entirely.
>>
>>> 2. According to ACPI IORT spec, named component specific data has a node 
>>> flags field
>>>whoes bit0 is for Stall support. However, it do not have any field for 
>>> pasid bit.
>>>Can we use other 5 bits[5:1] for pasid bit numbers, so we can have 32 
>>> pasid bit for
>>>a single platform device which should be enough, because SMMU only 
>>> support 20 bit pasid
>>>
> 
> Any comment on this?
> The ACPI IORT spec may need be updated?

I suppose that the Named Component Node could be used for SSID and stall
capability bits. Can't the ACPI namespace entries be extended to host
these capabilities in a more generic way? Platforms with different IOMMUs
might also need this information some day.

Thanks,
Jean
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH v7] vfio: platform: reset: Add Broadcom FlexRM reset module

2017-09-06 Thread Anup Patel via iommu
This patch adds Broadcom FlexRM low-level reset for
VFIO platform.

It will do the following:
1. Disable/Deactivate each FlexRM ring
2. Flush each FlexRM ring

The cleanup sequence for FlexRM rings is adapted from
Broadcom FlexRM mailbox driver.

Signed-off-by: Anup Patel 
Reviewed-by: Oza Oza 
Reviewed-by: Scott Branden 
Reviewed-by: Eric Auger 
---
 drivers/vfio/platform/reset/Kconfig|   9 ++
 drivers/vfio/platform/reset/Makefile   |   1 +
 .../vfio/platform/reset/vfio_platform_bcmflexrm.c  | 115 +
 3 files changed, 125 insertions(+)
 create mode 100644 drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c

diff --git a/drivers/vfio/platform/reset/Kconfig 
b/drivers/vfio/platform/reset/Kconfig
index 705..392e3c0 100644
--- a/drivers/vfio/platform/reset/Kconfig
+++ b/drivers/vfio/platform/reset/Kconfig
@@ -13,3 +13,12 @@ config VFIO_PLATFORM_AMDXGBE_RESET
  Enables the VFIO platform driver to handle reset for AMD XGBE
 
  If you don't know what to do here, say N.
+
+config VFIO_PLATFORM_BCMFLEXRM_RESET
+   tristate "VFIO support for Broadcom FlexRM reset"
+   depends on VFIO_PLATFORM && (ARCH_BCM_IPROC || COMPILE_TEST)
+   default ARCH_BCM_IPROC
+   help
+ Enables the VFIO platform driver to handle reset for Broadcom FlexRM
+
+ If you don't know what to do here, say N.
diff --git a/drivers/vfio/platform/reset/Makefile 
b/drivers/vfio/platform/reset/Makefile
index 93f4e23..8d9874b 100644
--- a/drivers/vfio/platform/reset/Makefile
+++ b/drivers/vfio/platform/reset/Makefile
@@ -5,3 +5,4 @@ ccflags-y += -Idrivers/vfio/platform
 
 obj-$(CONFIG_VFIO_PLATFORM_CALXEDAXGMAC_RESET) += vfio-platform-calxedaxgmac.o
 obj-$(CONFIG_VFIO_PLATFORM_AMDXGBE_RESET) += vfio-platform-amdxgbe.o
+obj-$(CONFIG_VFIO_PLATFORM_BCMFLEXRM_RESET) += vfio_platform_bcmflexrm.o
diff --git a/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c 
b/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
new file mode 100644
index 000..5f89066
--- /dev/null
+++ b/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
@@ -0,0 +1,115 @@
+/*
+ * Copyright (C) 2017 Broadcom
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see .
+ */
+
+/*
+ * This driver provides reset support for Broadcom FlexRM ring manager
+ * to VFIO platform.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "vfio_platform_private.h"
+
+/* FlexRM configuration */
+#define RING_REGS_SIZE 0x1
+#define RING_VER_MAGIC 0x76303031
+
+/* Per-Ring register offsets */
+#define RING_VER   0x000
+#define RING_CONTROL   0x034
+#define RING_FLUSH_DONE0x038
+
+/* Register RING_CONTROL fields */
+#define CONTROL_FLUSH_SHIFT5
+
+/* Register RING_FLUSH_DONE fields */
+#define FLUSH_DONE_MASK0x1
+
+static int vfio_platform_bcmflexrm_shutdown(void __iomem *ring)
+{
+   unsigned int timeout;
+
+   /* Disable/inactivate ring */
+   writel_relaxed(0x0, ring + RING_CONTROL);
+
+   /* Set ring flush state */
+   timeout = 1000; /* timeout of 1s */
+   writel_relaxed(BIT(CONTROL_FLUSH_SHIFT), ring + RING_CONTROL);
+   do {
+   if (readl_relaxed(ring + RING_FLUSH_DONE) &
+   FLUSH_DONE_MASK)
+   break;
+   mdelay(1);
+   } while (--timeout);
+   if (!timeout)
+   return -ETIMEDOUT;
+
+   /* Clear ring flush state */
+   timeout = 1000; /* timeout of 1s */
+   writel_relaxed(0x0, ring + RING_CONTROL);
+   do {
+   if (!(readl_relaxed(ring + RING_FLUSH_DONE) &
+ FLUSH_DONE_MASK))
+   break;
+   mdelay(1);
+   } while (--timeout);
+   if (!timeout)
+   return -ETIMEDOUT;
+
+   return 0;
+}
+
+static int vfio_platform_bcmflexrm_reset(struct vfio_platform_device *vdev)
+{
+   void __iomem *ring;
+   int rc = 0, ret = 0, ring_num = 0;
+   struct vfio_platform_region *reg = &vdev->regions[0];
+
+   /* Map FlexRM ring registers if not mapped */
+   if (!reg->ioaddr) {
+   reg->ioaddr = ioremap_nocache(reg->addr, reg->size);
+   if (!reg->ioaddr)
+  

[PATCH v7] FlexRM support in VFIO platform

2017-09-06 Thread Anup Patel via iommu
This patchset primarily adds Broadcom FlexRM reset module for
VFIO platform driver.

The patches are based on Linux-4.13-rc3 and can also be
found at flexrm-vfio-v7 branch of
https://github.com/Broadcom/arm64-linux.git

Changes since v6:
 - Update the FlexRM ring flush sequence as suggested
   by HW folks
 - Shutdown all FlexRM ring anyway even if it fails on
   any of the FlexRM rings
 - Use dev_warn() instead of pr_warn()

Changes since v5:
 - Make kconfig option VFIO_PLATFORM_BCMFLEXRM_RESET
   default to ARCH_BCM_IPROC

Changes since v4:
 - Use "--timeout" instead of "timeout--" in
   vfio_platform_bcmflexrm_shutdown()

Changes since v3:
 - Improve "depends on" for Kconfig option
   VFIO_PLATFORM_BCMFLEXRM_RESET
 - Fix typo in pr_warn() called by
   vfio_platform_bcmflexrm_shutdown()
 - Return error from vfio_platform_bcmflexrm_shutdown()
   when FlexRM ring flush timeout happens

Changes since v2:
 - Remove PATCH1 because fixing VFIO no-IOMMU mode is
   a separate topic

Changes since v1:
 - Remove iommu_present() check in vfio_iommu_group_get()
 - Drop PATCH1-to-PATCH3 because IOMMU_CAP_BYPASS is not
   required
 - Move additional comments out of license header in
   vfio_platform_bcmflexrm.c

Anup Patel (1):
  vfio: platform: reset: Add Broadcom FlexRM reset module

 drivers/vfio/platform/reset/Kconfig|   9 ++
 drivers/vfio/platform/reset/Makefile   |   1 +
 .../vfio/platform/reset/vfio_platform_bcmflexrm.c  | 115 +
 3 files changed, 125 insertions(+)
 create mode 100644 drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c

-- 
2.7.4

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v6] vfio: platform: reset: Add Broadcom FlexRM reset module

2017-09-06 Thread Anup Patel via iommu
On Tue, Sep 5, 2017 at 9:27 PM, Alex Williamson
 wrote:
> On Mon, 4 Sep 2017 15:20:11 +0530
> Anup Patel  wrote:
>
>> Sorry for delayed response...
>>
>> On Tue, Aug 29, 2017 at 7:39 PM, Konrad Rzeszutek Wilk
>>  wrote:
>> > On Tue, Aug 29, 2017 at 09:34:46AM +0530, Anup Patel wrote:
>> >> This patch adds Broadcom FlexRM low-level reset for
>> >> VFIO platform.
>> >>
>> >
>> > Is there an document that explains and /or details the various
>> > registers?
>>
>> Yes, there is a document but its not publicly accessible.
>>
>> >> It will do the following:
>> >> 1. Disable/Deactivate each FlexRM ring
>> >> 2. Flush each FlexRM ring
>> >>
>> >> The cleanup sequence for FlexRM rings is adapted from
>> >> Broadcom FlexRM mailbox driver.
>> >>
>> >> Signed-off-by: Anup Patel 
>> >> Reviewed-by: Oza Oza 
>> >> Reviewed-by: Scott Branden 
>> >> Reviewed-by: Eric Auger 
>> >> ---
>> >>  drivers/vfio/platform/reset/Kconfig|   9 ++
>> >>  drivers/vfio/platform/reset/Makefile   |   1 +
>> >>  .../vfio/platform/reset/vfio_platform_bcmflexrm.c  | 100 
>> >> +
>> >>  3 files changed, 110 insertions(+)
>> >>  create mode 100644 drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
>> >>
>> >> diff --git a/drivers/vfio/platform/reset/Kconfig 
>> >> b/drivers/vfio/platform/reset/Kconfig
>> >> index 705..392e3c0 100644
>> >> --- a/drivers/vfio/platform/reset/Kconfig
>> >> +++ b/drivers/vfio/platform/reset/Kconfig
>> >> @@ -13,3 +13,12 @@ config VFIO_PLATFORM_AMDXGBE_RESET
>> >> Enables the VFIO platform driver to handle reset for AMD XGBE
>> >>
>> >> If you don't know what to do here, say N.
>> >> +
>> >> +config VFIO_PLATFORM_BCMFLEXRM_RESET
>> >> + tristate "VFIO support for Broadcom FlexRM reset"
>> >> + depends on VFIO_PLATFORM && (ARCH_BCM_IPROC || COMPILE_TEST)
>> >> + default ARCH_BCM_IPROC
>> >> + help
>> >> +   Enables the VFIO platform driver to handle reset for Broadcom 
>> >> FlexRM
>> >> +
>> >> +   If you don't know what to do here, say N.
>> >> diff --git a/drivers/vfio/platform/reset/Makefile 
>> >> b/drivers/vfio/platform/reset/Makefile
>> >> index 93f4e23..8d9874b 100644
>> >> --- a/drivers/vfio/platform/reset/Makefile
>> >> +++ b/drivers/vfio/platform/reset/Makefile
>> >> @@ -5,3 +5,4 @@ ccflags-y += -Idrivers/vfio/platform
>> >>
>> >>  obj-$(CONFIG_VFIO_PLATFORM_CALXEDAXGMAC_RESET) += 
>> >> vfio-platform-calxedaxgmac.o
>> >>  obj-$(CONFIG_VFIO_PLATFORM_AMDXGBE_RESET) += vfio-platform-amdxgbe.o
>> >> +obj-$(CONFIG_VFIO_PLATFORM_BCMFLEXRM_RESET) += vfio_platform_bcmflexrm.o
>> >> diff --git a/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c 
>> >> b/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
>> >> new file mode 100644
>> >> index 000..966a813
>> >> --- /dev/null
>> >> +++ b/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
>> >> @@ -0,0 +1,100 @@
>> >> +/*
>> >> + * Copyright (C) 2017 Broadcom
>> >> + *
>> >> + * 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.
>> >> + *
>> >> + * You should have received a copy of the GNU General Public License
>> >> + * along with this program.  If not, see .
>> >> + */
>> >> +
>> >> +/*
>> >> + * This driver provides reset support for Broadcom FlexRM ring manager
>> >> + * to VFIO platform.
>> >> + */
>> >> +
>> >> +#include 
>> >> +#include 
>> >> +#include 
>> >> +#include 
>> >> +#include 
>> >> +
>> >> +#include "vfio_platform_private.h"
>> >> +
>> >> +/* FlexRM configuration */
>> >> +#define RING_REGS_SIZE   0x1
>> >> +#define RING_VER_MAGIC   0x76303031
>> >> +
>> >> +/* Per-Ring register offsets */
>> >> +#define RING_VER 0x000
>> >> +#define RING_CONTROL 0x034
>> >> +#define RING_FLUSH_DONE  0x038
>> >> +
>> >> +/* Register RING_CONTROL fields */
>> >> +#define CONTROL_FLUSH_SHIFT  5
>> >> +
>> >> +/* Register RING_FLUSH_DONE fields */
>> >> +#define FLUSH_DONE_MASK  0x1
>> >> +
>> >> +static int vfio_platform_bcmflexrm_shutdown(void __iomem *ring)
>> >> +{
>> >> + unsigned int timeout;
>> >> +
>> >> + /* Disable/inactivate ring */
>> >> + writel_relaxed(0x0, ring + RING_CONTROL);
>> >> +
>> >> + /* Flush ring with timeout of 1s */
>> >> + timeout = 1000;
>> >
>> > Perhaps a #define for this value?
>>
>> This magic value 1000 makes it explic