Re: [PATCH v7 00/12] Fix kdump faults on system with amd iommu

2016-12-23 Thread Baoquan He
Hi Joerg,

Ping!

Could you help review this version? Not sure this could catch up to
v4.10 merging.

Thanks
Baoqaun

On 11/25/16 at 01:13pm, Baoquan He wrote:
> This is v7 post.
> 
> The principle of the fix is similar to intel iommu. Just defer the assignment
> of device to domain to device driver init. In this version of post, a new
> call-back is_attach_deferred is added to iommu-ops, it's used to check whether
> we need defer the domain attach/detach in iommu-core code.
> 
> v5:
> bnx2 NIC can't reset itself during driver init. Post patch to reset
> it during driver init. IO_PAGE_FAULT can't be seen anymore.
> 
> Below is link of v5 post.
> 
> https://lists.linuxfoundation.org/pipermail/iommu/2016-September/018527.html
> 
> v5->v6:
> According to Joerg's comments made several below main changes:
> - Add sanity check when copy old dev tables.
> 
> - If a device is set up with guest translations (DTE.GV=1), then don't
>   copy that information but move the device over to an empty guest-cr3
>   table and handle the faults in the PPR log (which just answer them
>   with INVALID).
> 
> v6->v7:
> Two main changes are made according to Joerg's suggestion:
> - Add is_attach_deferred call-back to iommu-ops. With this domain
>   can be deferred to device driver init cleanly.
> 
> - Allocate memory below 4G for dev table if translation pre-enabled.
>   AMD engineer pointed out that it's unsafe to update the device-table
>   while iommu is enabled. device-table pointer update is split up into
>   two 32bit writes in the IOMMU hardware. So updating it while the IOMMU
>   is enabled could have some nasty side effects.
> 
> Baoquan He (12):
>   iommu/amd: Detect pre enabled translation
>   iommu/amd: add several helper function
>   iommu/amd: Define bit fields for DTE particularly
>   iommu/amd: Add function copy_dev_tables
>   iommu/amd: copy old trans table from old kernel
>   iommu: Add is_attach_deferred call-back to iommu-ops
>   iommu/amd: Use is_attach_deferred call-back
>   iommu/amd: Add sanity check of irq remap information of old dev table
> entry
>   iommu/amd: Don't copy GCR3 table root pointer
>   iommu/amd: Clear out the GV flag when handle deferred domain attach
>   iommu: Assign the direct mapped domain to group->domain
>   iommu/amd: Allocate memory below 4G for dev table if translation
> pre-enabled
> 
>  drivers/iommu/amd_iommu.c   |  78 +---
>  drivers/iommu/amd_iommu_init.c  | 201 
> +---
>  drivers/iommu/amd_iommu_proto.h |   2 +
>  drivers/iommu/amd_iommu_types.h |  53 ++-
>  drivers/iommu/amd_iommu_v2.c|  18 +++-
>  drivers/iommu/iommu.c   |   9 ++
>  include/linux/iommu.h   |   1 +
>  7 files changed, 313 insertions(+), 49 deletions(-)
> 
> -- 
> 2.5.5
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [RFC v4 15/16] vfio/type1: Check MSI remapping at irq domain level

2016-12-23 Thread Auger Eric
Hi Geetha,

On 23/12/2016 14:33, Geetha Akula wrote:
> Hi Eric,
> 
> Seeing same issue reported by Diana on ThunderX with you
> v4.9-reserved-v4 branch.
> Vfio passthough work fine when allow_unsafe_interrupts is set.
Thank you for testing! I will fix the security assessment by better
studying flag propagation in domain hierarchy.

Best Regards

Eric

> 
> 
> Thank you,
> Geetha.
> 
> On Thu, Dec 22, 2016 at 6:32 PM, Auger Eric  > wrote:
> 
> Hi Diana,
> 
> On 22/12/2016 13:41, Diana Madalina Craciun wrote:
> > Hi Eric,
> >
> > On 12/13/2016 10:32 PM, Eric Auger wrote:
> >> In case the IOMMU does not bypass MSI transactions (typical
> >> case on ARM), we check all MSI controllers are IRQ remapping
> >> capable. If not the IRQ assignment may be unsafe.
> >>
> >> At this stage the arm-smmu-(v3) still advertise the
> >> IOMMU_CAP_INTR_REMAP capability at IOMMU level. This will be
> >> removed in subsequent patches.
> >>
> >> Signed-off-by: Eric Auger  >
> >> ---
> >>  drivers/vfio/vfio_iommu_type1.c | 9 ++---
> >>  1 file changed, 6 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/vfio/vfio_iommu_type1.c
> b/drivers/vfio/vfio_iommu_type1.c
> >> index d07fe73..a05648b 100644
> >> --- a/drivers/vfio/vfio_iommu_type1.c
> >> +++ b/drivers/vfio/vfio_iommu_type1.c
> >> @@ -37,6 +37,7 @@
> >>  #include 
> >>  #include 
> >>  #include 
> >> +#include 
> >>
> >>  #define DRIVER_VERSION  "0.2"
> >>  #define DRIVER_AUTHOR   "Alex Williamson
> >"
> >> @@ -765,7 +766,7 @@ static int vfio_iommu_type1_attach_group(void
> *iommu_data,
> >>  struct vfio_domain *domain, *d;
> >>  struct bus_type *bus = NULL;
> >>  int ret;
> >> -bool resv_msi;
> >> +bool resv_msi, msi_remap;
> >>  phys_addr_t resv_msi_base;
> >>
> >>  mutex_lock(>lock);
> >> @@ -818,8 +819,10 @@ static int
> vfio_iommu_type1_attach_group(void *iommu_data,
> >>  INIT_LIST_HEAD(>group_list);
> >>  list_add(>next, >group_list);
> >>
> >> -if (!allow_unsafe_interrupts &&
> >> -!iommu_capable(bus, IOMMU_CAP_INTR_REMAP)) {
> >> +msi_remap = resv_msi ? irq_domain_check_msi_remap() :
> >> +   iommu_capable(bus, IOMMU_CAP_INTR_REMAP);
> >> +
> >> +if (!allow_unsafe_interrupts && !msi_remap) {
> >>  pr_warn("%s: No interrupt remapping support.  Use
> the module param \"allow_unsafe_interrupts\" to enable VFIO IOMMU
> support on this platform\n",
> >> __func__);
> >>  ret = -EPERM;
> >
> > I tested your v4.9-reserved-v4 branch on a ITS capable hardware (NXP
> > LS2080), so I did not set allow_unsafe_interrupts. It fails here
> > complaining that the there is no interrupt remapping support. The
> > irq_domain_check_msi_remap function returns false as none of the
> checked
> > domains has the IRQ_DOMAIN_FLAG_MSI_REMAP flag set. I think the reason
> > is that the flags are not propagated through the domain hierarchy when
> > the domain is created.
> 
> Hum OK. Please apologize for the inconvenience, all the more so this is
> the second time you report the same issue for different cause :-( At the
> moment I can't test on a GICv3 ITS based system. I will try to fix that
> though.
> 
> I would like to get the confirmation introducing this flag is the right
> direction though.
> 
> Thanks
> 
> Eric
> >
> > Thanks,
> >
> > Diana
> >
> >
> >
> 
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> 
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 
> 
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [RFC v4 15/16] vfio/type1: Check MSI remapping at irq domain level

2016-12-23 Thread Geetha Akula
Hi Eric,

Seeing same issue reported by Diana on ThunderX with you v4.9-reserved-v4
branch.
Vfio passthough work fine when allow_unsafe_interrupts is set.


Thank you,
Geetha.

On Thu, Dec 22, 2016 at 6:32 PM, Auger Eric  wrote:

> Hi Diana,
>
> On 22/12/2016 13:41, Diana Madalina Craciun wrote:
> > Hi Eric,
> >
> > On 12/13/2016 10:32 PM, Eric Auger wrote:
> >> In case the IOMMU does not bypass MSI transactions (typical
> >> case on ARM), we check all MSI controllers are IRQ remapping
> >> capable. If not the IRQ assignment may be unsafe.
> >>
> >> At this stage the arm-smmu-(v3) still advertise the
> >> IOMMU_CAP_INTR_REMAP capability at IOMMU level. This will be
> >> removed in subsequent patches.
> >>
> >> Signed-off-by: Eric Auger 
> >> ---
> >>  drivers/vfio/vfio_iommu_type1.c | 9 ++---
> >>  1 file changed, 6 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_
> type1.c
> >> index d07fe73..a05648b 100644
> >> --- a/drivers/vfio/vfio_iommu_type1.c
> >> +++ b/drivers/vfio/vfio_iommu_type1.c
> >> @@ -37,6 +37,7 @@
> >>  #include 
> >>  #include 
> >>  #include 
> >> +#include 
> >>
> >>  #define DRIVER_VERSION  "0.2"
> >>  #define DRIVER_AUTHOR   "Alex Williamson "
> >> @@ -765,7 +766,7 @@ static int vfio_iommu_type1_attach_group(void
> *iommu_data,
> >>  struct vfio_domain *domain, *d;
> >>  struct bus_type *bus = NULL;
> >>  int ret;
> >> -bool resv_msi;
> >> +bool resv_msi, msi_remap;
> >>  phys_addr_t resv_msi_base;
> >>
> >>  mutex_lock(>lock);
> >> @@ -818,8 +819,10 @@ static int vfio_iommu_type1_attach_group(void
> *iommu_data,
> >>  INIT_LIST_HEAD(>group_list);
> >>  list_add(>next, >group_list);
> >>
> >> -if (!allow_unsafe_interrupts &&
> >> -!iommu_capable(bus, IOMMU_CAP_INTR_REMAP)) {
> >> +msi_remap = resv_msi ? irq_domain_check_msi_remap() :
> >> +   iommu_capable(bus, IOMMU_CAP_INTR_REMAP);
> >> +
> >> +if (!allow_unsafe_interrupts && !msi_remap) {
> >>  pr_warn("%s: No interrupt remapping support.  Use the
> module param \"allow_unsafe_interrupts\" to enable VFIO IOMMU support on
> this platform\n",
> >> __func__);
> >>  ret = -EPERM;
> >
> > I tested your v4.9-reserved-v4 branch on a ITS capable hardware (NXP
> > LS2080), so I did not set allow_unsafe_interrupts. It fails here
> > complaining that the there is no interrupt remapping support. The
> > irq_domain_check_msi_remap function returns false as none of the checked
> > domains has the IRQ_DOMAIN_FLAG_MSI_REMAP flag set. I think the reason
> > is that the flags are not propagated through the domain hierarchy when
> > the domain is created.
>
> Hum OK. Please apologize for the inconvenience, all the more so this is
> the second time you report the same issue for different cause :-( At the
> moment I can't test on a GICv3 ITS based system. I will try to fix that
> though.
>
> I would like to get the confirmation introducing this flag is the right
> direction though.
>
> Thanks
>
> Eric
> >
> > Thanks,
> >
> > Diana
> >
> >
> >
>
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH V6 3/7] perf/amd/iommu: Modify IOMMU API to allow specifying IOMMU index

2016-12-23 Thread Suravee Suthikulpanit
The current amd_iommu_pc_get_set_reg_val() cannot support multi-IOMMU.
It is also confusing since it is trying to support set and get in
one function.

So, this patch breaks it down to amd_iommu_pc_[get|set]_counter(),
and modifies them to allow callers to specify IOMMU index. This prepares
the driver for supporting multi-IOMMU in subsequent patch.

Cc: Peter Zijlstra 
Cc: Borislav Petkov 
Cc: Joerg Roedel 
Signed-off-by: Suravee Suthikulpanit 
---
 arch/x86/events/amd/iommu.c | 45 +++-
 arch/x86/events/amd/iommu.h |  8 +++--
 drivers/iommu/amd_iommu_init.c  | 77 +++--
 drivers/iommu/amd_iommu_proto.h | 10 --
 4 files changed, 93 insertions(+), 47 deletions(-)

diff --git a/arch/x86/events/amd/iommu.c b/arch/x86/events/amd/iommu.c
index cf94f48..9744dc8 100644
--- a/arch/x86/events/amd/iommu.c
+++ b/arch/x86/events/amd/iommu.c
@@ -248,46 +248,44 @@ static void perf_iommu_enable_event(struct perf_event *ev)
 {
u8 csource = _GET_CSOURCE(ev);
u16 devid = _GET_DEVID(ev);
+   u8 bank = _GET_BANK(ev);
+   u8 cntr = _GET_CNTR(ev);
u64 reg = 0ULL;
 
reg = csource;
-   amd_iommu_pc_get_set_reg_val(devid,
-   _GET_BANK(ev), _GET_CNTR(ev) ,
-IOMMU_PC_COUNTER_SRC_REG, , true);
+   amd_iommu_pc_set_reg(0, devid, bank, cntr,
+IOMMU_PC_COUNTER_SRC_REG, );
 
reg = 0ULL | devid | (_GET_DEVID_MASK(ev) << 32);
if (reg)
reg |= (1UL << 31);
-   amd_iommu_pc_get_set_reg_val(devid,
-   _GET_BANK(ev), _GET_CNTR(ev) ,
-IOMMU_PC_DEVID_MATCH_REG, , true);
+   amd_iommu_pc_set_reg(0, devid, bank, cntr,
+IOMMU_PC_DEVID_MATCH_REG, );
 
reg = 0ULL | _GET_PASID(ev) | (_GET_PASID_MASK(ev) << 32);
if (reg)
reg |= (1UL << 31);
-   amd_iommu_pc_get_set_reg_val(devid,
-   _GET_BANK(ev), _GET_CNTR(ev) ,
-IOMMU_PC_PASID_MATCH_REG, , true);
+   amd_iommu_pc_set_reg(0, devid, bank, cntr,
+IOMMU_PC_PASID_MATCH_REG, );
 
reg = 0ULL | _GET_DOMID(ev) | (_GET_DOMID_MASK(ev) << 32);
if (reg)
reg |= (1UL << 31);
-   amd_iommu_pc_get_set_reg_val(devid,
-   _GET_BANK(ev), _GET_CNTR(ev) ,
-IOMMU_PC_DOMID_MATCH_REG, , true);
+   amd_iommu_pc_set_reg(0, devid, bank, cntr,
+IOMMU_PC_DOMID_MATCH_REG, );
 }
 
 static void perf_iommu_disable_event(struct perf_event *event)
 {
u64 reg = 0ULL;
 
-   amd_iommu_pc_get_set_reg_val(_GET_DEVID(event),
-   _GET_BANK(event), _GET_CNTR(event),
-   IOMMU_PC_COUNTER_SRC_REG, , true);
+   amd_iommu_pc_set_reg(0, _GET_DEVID(event), _GET_BANK(event),
+_GET_CNTR(event), IOMMU_PC_COUNTER_SRC_REG, );
 }
 
 static void perf_iommu_start(struct perf_event *event, int flags)
 {
+   u64 val;
struct hw_perf_event *hwc = >hw;
 
pr_debug("perf: amd_iommu:perf_iommu_start\n");
@@ -297,13 +295,13 @@ static void perf_iommu_start(struct perf_event *event, 
int flags)
WARN_ON_ONCE(!(hwc->state & PERF_HES_UPTODATE));
hwc->state = 0;
 
-   if (flags & PERF_EF_RELOAD) {
-   u64 prev_raw_count =  local64_read(>prev_count);
-   amd_iommu_pc_get_set_reg_val(_GET_DEVID(event),
-   _GET_BANK(event), _GET_CNTR(event),
-   IOMMU_PC_COUNTER_REG, _raw_count, true);
-   }
+   if (!(flags & PERF_EF_RELOAD))
+   goto enable;
+
+   val = local64_read(>prev_count);
 
+   amd_iommu_pc_set_counter(0, _GET_BANK(event), _GET_CNTR(event), );
+enable:
perf_iommu_enable_event(event);
perf_event_update_userpage(event);
 
@@ -316,9 +314,8 @@ static void perf_iommu_read(struct perf_event *event)
struct hw_perf_event *hwc = >hw;
pr_debug("perf: amd_iommu:perf_iommu_read\n");
 
-   amd_iommu_pc_get_set_reg_val(_GET_DEVID(event),
-   _GET_BANK(event), _GET_CNTR(event),
-   IOMMU_PC_COUNTER_REG, , false);
+   if (amd_iommu_pc_get_counter(0, _GET_BANK(event), _GET_CNTR(event), 
))
+   return;
 
/* IOMMU pc counter register is only 48 bits */
cnt &= GENMASK_ULL(48, 0);
diff --git a/arch/x86/events/amd/iommu.h b/arch/x86/events/amd/iommu.h
index 0ce8c91..71ec9d7 100644
--- a/arch/x86/events/amd/iommu.h
+++ b/arch/x86/events/amd/iommu.h
@@ -31,7 +31,11 @@
 
 extern u8 amd_iommu_pc_get_max_counters(uint devid);
 
-extern int amd_iommu_pc_get_set_reg_val(u16 devid, u8 bank, u8 cntr,
-   u8 fxn, u64 *value, bool 

[PATCH V6 5/7] perf/amd/iommu: Clean up perf_iommu_enable_event

2016-12-23 Thread Suravee Suthikulpanit
This patch cleans up:
  * Various bitwise operations in perf_iommu_enable_event
  * Make use macros BIT(x)

This should not affect logic and functionality.

Cc: Peter Zijlstra 
Cc: Borislav Petkov 
Signed-off-by: Suravee Suthikulpanit 
---
 arch/x86/events/amd/iommu.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/x86/events/amd/iommu.c b/arch/x86/events/amd/iommu.c
index 9bff41d..2403c78 100644
--- a/arch/x86/events/amd/iommu.c
+++ b/arch/x86/events/amd/iommu.c
@@ -258,21 +258,21 @@ static void perf_iommu_enable_event(struct perf_event *ev)
amd_iommu_pc_set_reg(0, devid, bank, cntr,
 IOMMU_PC_COUNTER_SRC_REG, );
 
-   reg = 0ULL | devid | (_GET_DEVID_MASK(ev) << 32);
+   reg = devid | (_GET_DEVID_MASK(ev) << 32);
if (reg)
-   reg |= (1UL << 31);
+   reg |= BIT(31);
amd_iommu_pc_set_reg(0, devid, bank, cntr,
 IOMMU_PC_DEVID_MATCH_REG, );
 
-   reg = 0ULL | _GET_PASID(ev) | (_GET_PASID_MASK(ev) << 32);
+   reg = _GET_PASID(ev) | (_GET_PASID_MASK(ev) << 32);
if (reg)
-   reg |= (1UL << 31);
+   reg |= BIT(31);
amd_iommu_pc_set_reg(0, devid, bank, cntr,
 IOMMU_PC_PASID_MATCH_REG, );
 
-   reg = 0ULL | _GET_DOMID(ev) | (_GET_DOMID_MASK(ev) << 32);
+   reg = _GET_DOMID(ev) | (_GET_DOMID_MASK(ev) << 32);
if (reg)
-   reg |= (1UL << 31);
+   reg |= BIT(31);
amd_iommu_pc_set_reg(0, devid, bank, cntr,
 IOMMU_PC_DOMID_MATCH_REG, );
 }
-- 
1.8.3.1

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


[PATCH V6 4/7] perf/amd/iommu: Declare pr_fmt and remove unnecessary pr_debug

2016-12-23 Thread Suravee Suthikulpanit
This patch declare pr_fmt for perf/amd_iommu and remove unnecessary
pr_debug.

Cc: Peter Zijlstra 
Cc: Borislav Petkov 
Signed-off-by: Suravee Suthikulpanit 
---
 arch/x86/events/amd/iommu.c | 12 
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/arch/x86/events/amd/iommu.c b/arch/x86/events/amd/iommu.c
index 9744dc8..9bff41d 100644
--- a/arch/x86/events/amd/iommu.c
+++ b/arch/x86/events/amd/iommu.c
@@ -11,6 +11,8 @@
  * published by the Free Software Foundation.
  */
 
+#define pr_fmt(fmt)"perf/amd_iommu: " fmt
+
 #include 
 #include 
 #include 
@@ -288,7 +290,6 @@ static void perf_iommu_start(struct perf_event *event, int 
flags)
u64 val;
struct hw_perf_event *hwc = >hw;
 
-   pr_debug("perf: amd_iommu:perf_iommu_start\n");
if (WARN_ON_ONCE(!(hwc->state & PERF_HES_STOPPED)))
return;
 
@@ -312,7 +313,6 @@ static void perf_iommu_read(struct perf_event *event)
u64 cnt, prev;
s64 delta;
struct hw_perf_event *hwc = >hw;
-   pr_debug("perf: amd_iommu:perf_iommu_read\n");
 
if (amd_iommu_pc_get_counter(0, _GET_BANK(event), _GET_CNTR(event), 
))
return;
@@ -339,8 +339,6 @@ static void perf_iommu_stop(struct perf_event *event, int 
flags)
struct hw_perf_event *hwc = >hw;
u64 config;
 
-   pr_debug("perf: amd_iommu:perf_iommu_stop\n");
-
if (hwc->state & PERF_HES_UPTODATE)
return;
 
@@ -362,7 +360,6 @@ static int perf_iommu_add(struct perf_event *event, int 
flags)
struct perf_amd_iommu *perf_iommu =
container_of(event->pmu, struct perf_amd_iommu, pmu);
 
-   pr_debug("perf: amd_iommu:perf_iommu_add\n");
event->hw.state = PERF_HES_UPTODATE | PERF_HES_STOPPED;
 
/* request an iommu bank/counter */
@@ -383,7 +380,6 @@ static void perf_iommu_del(struct perf_event *event, int 
flags)
struct perf_amd_iommu *perf_iommu =
container_of(event->pmu, struct perf_amd_iommu, pmu);
 
-   pr_debug("perf: amd_iommu:perf_iommu_del\n");
perf_iommu_stop(event, PERF_EF_UPDATE);
 
/* clear the assigned iommu bank/counter */
@@ -443,7 +439,7 @@ static __init int _init_perf_amd_iommu(
 
/* Init events attributes */
if (_init_events_attrs(perf_iommu) != 0)
-   pr_err("perf: amd_iommu: Only support raw events.\n");
+   pr_err("Only support raw events.\n");
 
perf_iommu->max_banks = amd_iommu_pc_get_max_banks(0);
perf_iommu->max_counters = amd_iommu_pc_get_max_counters(0);
@@ -456,7 +452,7 @@ static __init int _init_perf_amd_iommu(
 
ret = perf_pmu_register(_iommu->pmu, name, -1);
if (ret) {
-   pr_err("perf: amd_iommu: Failed to initialized.\n");
+   pr_err("Error initializing AMD IOMMU perf counters.\n");
amd_iommu_pc_exit();
} else {
pr_info("perf: amd_iommu: Detected. (%d banks, %d 
counters/bank)\n",
-- 
1.8.3.1

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


[PATCH V6 7/7] perf/amd/iommu: Enable support for multiple IOMMUs

2016-12-23 Thread Suravee Suthikulpanit
This patch adds multi-IOMMU support for perf by exposing
an AMD IOMMU PMU for each IOMMU found in the system via:

  /sys/device/amd_iommu_x /* where x is the IOMMU index. */

This allows users to specify different events to be programed
onto performance counters of each IOMMU.

Cc: Peter Zijlstra 
Cc: Borislav Petkov 
Signed-off-by: Suravee Suthikulpanit 
---
 arch/x86/events/amd/iommu.c | 119 
 1 file changed, 64 insertions(+), 55 deletions(-)

diff --git a/arch/x86/events/amd/iommu.c b/arch/x86/events/amd/iommu.c
index 2403c78..5fd97b5 100644
--- a/arch/x86/events/amd/iommu.c
+++ b/arch/x86/events/amd/iommu.c
@@ -35,10 +35,13 @@
 #define _GET_PASID_MASK(ev) ((ev->hw.extra_reg.config >> 16) & 0xULL)
 #define _GET_DOMID_MASK(ev) ((ev->hw.extra_reg.config >> 32) & 0xULL)
 
-static struct perf_amd_iommu __perf_iommu;
+#define PERF_AMD_IOMMU_NAME_SZ 16
 
 struct perf_amd_iommu {
+   struct list_head list;
struct pmu pmu;
+   uint idx;
+   char name[PERF_AMD_IOMMU_NAME_SZ];
u8 max_banks;
u8 max_counters;
u64 cntr_assign_mask;
@@ -46,6 +49,8 @@ struct perf_amd_iommu {
const struct attribute_group *attr_groups[4];
 };
 
+LIST_HEAD(perf_amd_iommu_list);
+
 #define format_group   attr_groups[0]
 #define cpumask_group  attr_groups[1]
 #define events_group   attr_groups[2]
@@ -204,8 +209,7 @@ static int clear_avail_iommu_bnk_cntr(struct perf_amd_iommu 
*perf_iommu,
 static int perf_iommu_event_init(struct perf_event *event)
 {
struct hw_perf_event *hwc = >hw;
-   struct perf_amd_iommu *perf_iommu;
-   u64 config, config1;
+   struct perf_amd_iommu *pi = container_of(event->pmu, struct 
perf_amd_iommu, pmu);
 
/* test the event attr type check for PMU enumeration */
if (event->attr.type != event->pmu->type)
@@ -227,27 +231,17 @@ static int perf_iommu_event_init(struct perf_event *event)
if (event->cpu < 0)
return -EINVAL;
 
-   perf_iommu = &__perf_iommu;
-
-   if (event->pmu != _iommu->pmu)
-   return -ENOENT;
-
-   if (perf_iommu) {
-   config = event->attr.config;
-   config1 = event->attr.config1;
-   } else {
-   return -EINVAL;
-   }
-
/* update the hw_perf_event struct with the iommu config data */
-   hwc->config = config;
-   hwc->extra_reg.config = config1;
+   hwc->idx = pi->idx;
+   hwc->config = event->attr.config;
+   hwc->extra_reg.config = event->attr.config1;
 
return 0;
 }
 
 static void perf_iommu_enable_event(struct perf_event *ev)
 {
+   struct hw_perf_event *hwc = >hw;
u8 csource = _GET_CSOURCE(ev);
u16 devid = _GET_DEVID(ev);
u8 bank = _GET_BANK(ev);
@@ -255,33 +249,34 @@ static void perf_iommu_enable_event(struct perf_event *ev)
u64 reg = 0ULL;
 
reg = csource;
-   amd_iommu_pc_set_reg(0, devid, bank, cntr,
+   amd_iommu_pc_set_reg(hwc->idx, devid, bank, cntr,
 IOMMU_PC_COUNTER_SRC_REG, );
 
reg = devid | (_GET_DEVID_MASK(ev) << 32);
if (reg)
reg |= BIT(31);
-   amd_iommu_pc_set_reg(0, devid, bank, cntr,
+   amd_iommu_pc_set_reg(hwc->idx, devid, bank, cntr,
 IOMMU_PC_DEVID_MATCH_REG, );
 
reg = _GET_PASID(ev) | (_GET_PASID_MASK(ev) << 32);
if (reg)
reg |= BIT(31);
-   amd_iommu_pc_set_reg(0, devid, bank, cntr,
+   amd_iommu_pc_set_reg(hwc->idx, devid, bank, cntr,
 IOMMU_PC_PASID_MATCH_REG, );
 
reg = _GET_DOMID(ev) | (_GET_DOMID_MASK(ev) << 32);
if (reg)
reg |= BIT(31);
-   amd_iommu_pc_set_reg(0, devid, bank, cntr,
+   amd_iommu_pc_set_reg(hwc->idx, devid, bank, cntr,
 IOMMU_PC_DOMID_MATCH_REG, );
 }
 
 static void perf_iommu_disable_event(struct perf_event *event)
 {
+   struct hw_perf_event *hwc = >hw;
u64 reg = 0ULL;
 
-   amd_iommu_pc_set_reg(0, _GET_DEVID(event), _GET_BANK(event),
+   amd_iommu_pc_set_reg(hwc->idx, _GET_DEVID(event), _GET_BANK(event),
 _GET_CNTR(event), IOMMU_PC_COUNTER_SRC_REG, );
 }
 
@@ -301,7 +296,7 @@ static void perf_iommu_start(struct perf_event *event, int 
flags)
 
val = local64_read(>prev_count);
 
-   amd_iommu_pc_set_counter(0, _GET_BANK(event), _GET_CNTR(event), );
+   amd_iommu_pc_set_counter(hwc->idx, _GET_BANK(event), _GET_CNTR(event), 
);
 enable:
perf_iommu_enable_event(event);
perf_event_update_userpage(event);
@@ -314,7 +309,7 @@ static void perf_iommu_read(struct perf_event *event)
s64 delta;
struct hw_perf_event *hwc = >hw;
 
-   if (amd_iommu_pc_get_counter(0, _GET_BANK(event), _GET_CNTR(event), 
))
+   if 

[PATCH V6 6/7] iommu/amd: Introduce amd_iommu_get_num_iommus()

2016-12-23 Thread Suravee Suthikulpanit
This patch introduces amd_iommu_get_num_iommus(). This is intended for
Perf AMD IOMMU driver.

Cc: Joerg Roedel 
Signed-off-by: Suravee Suthikulpanit 
---
 arch/x86/events/amd/iommu.h | 2 ++
 drivers/iommu/amd_iommu_init.c  | 7 ++-
 drivers/iommu/amd_iommu_proto.h | 1 +
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/arch/x86/events/amd/iommu.h b/arch/x86/events/amd/iommu.h
index 71ec9d7..891c7c4 100644
--- a/arch/x86/events/amd/iommu.h
+++ b/arch/x86/events/amd/iommu.h
@@ -25,6 +25,8 @@
 #define PC_MAX_SPEC_CNTRS  16
 
 /* amd_iommu_init.c external support functions */
+extern int amd_iommu_get_num_iommus(void);
+
 extern bool amd_iommu_pc_supported(void);
 
 extern u8 amd_iommu_pc_get_max_banks(uint devid);
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index c993c77..c3740cb 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -1329,7 +1329,7 @@ static int __init init_iommu_one(struct amd_iommu *iommu, 
struct ivhd_header *h)
 
/* Add IOMMU to internal data structures */
list_add_tail(>list, _iommu_list);
-   iommu->index = amd_iommus_present++;
+   iommu->index = amd_iommus_present++;
 
if (unlikely(iommu->index >= MAX_IOMMUS)) {
WARN(1, "AMD-Vi: System has more IOMMUs than supported by this 
driver\n");
@@ -2717,6 +2717,11 @@ static struct amd_iommu *get_amd_iommu(uint idx)
return iommu;
 }
 
+int amd_iommu_get_num_iommus(void)
+{
+   return amd_iommus_present;
+}
+
 /
  *
  * IOMMU EFR Performance Counter support functionality. This code allows
diff --git a/drivers/iommu/amd_iommu_proto.h b/drivers/iommu/amd_iommu_proto.h
index 1df3ff7..58dfdbf 100644
--- a/drivers/iommu/amd_iommu_proto.h
+++ b/drivers/iommu/amd_iommu_proto.h
@@ -61,6 +61,7 @@ extern int amd_iommu_domain_set_gcr3(struct iommu_domain 
*dom, int pasid,
 #define IOMMU_PC_COUNTER_REG   0x00
 #endif
 
+extern int amd_iommu_get_num_iommus(void);
 extern bool amd_iommu_pc_supported(void);
 extern u8 amd_iommu_pc_get_max_banks(uint devid);
 extern u8 amd_iommu_pc_get_max_counters(uint devid);
-- 
1.8.3.1

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


[PATCH V6 2/7] perf/amd/iommu: Modify functions to query max banks and counters

2016-12-23 Thread Suravee Suthikulpanit
Currently, amd_iommu_pc_get_max_[banks|counters]() use end-point
device ID to locate an IOMMU and check the reported max banks/counters.
The logic assumes that the IOMMU_BASE_DEVID belongs to the first IOMMU,
and uses it to acquire a reference to the first IOMMU, which does not work
on certain systems. Instead, we modify the function to take IOMMU index,
and use it to query the corresponded AMD IOMMU instance.

Note that we currently hard-code the IOMMU index to 0, since the current
AMD IOMMU perf implementation only supports single IOMMU. Subsequent patch
will add support for multi-IOMMU, and will use proper IOMMU index.

Cc: Peter Zijlstra 
Cc: Borislav Petkov 
Cc: Joerg Roedel 
Signed-off-by: Suravee Suthikulpanit 
---
 arch/x86/events/amd/iommu.c | 17 +++--
 arch/x86/events/amd/iommu.h |  7 ++-
 drivers/iommu/amd_iommu_init.c  | 35 +--
 drivers/iommu/amd_iommu_proto.h |  4 ++--
 4 files changed, 32 insertions(+), 31 deletions(-)

diff --git a/arch/x86/events/amd/iommu.c b/arch/x86/events/amd/iommu.c
index f387baf..cf94f48 100644
--- a/arch/x86/events/amd/iommu.c
+++ b/arch/x86/events/amd/iommu.c
@@ -237,14 +237,6 @@ static int perf_iommu_event_init(struct perf_event *event)
return -EINVAL;
}
 
-   /* integrate with iommu base devid (), assume one iommu */
-   perf_iommu->max_banks =
-   amd_iommu_pc_get_max_banks(IOMMU_BASE_DEVID);
-   perf_iommu->max_counters =
-   amd_iommu_pc_get_max_counters(IOMMU_BASE_DEVID);
-   if ((perf_iommu->max_banks == 0) || (perf_iommu->max_counters == 0))
-   return -EINVAL;
-
/* update the hw_perf_event struct with the iommu config data */
hwc->config = config;
hwc->extra_reg.config = config1;
@@ -456,6 +448,11 @@ static __init int _init_perf_amd_iommu(
if (_init_events_attrs(perf_iommu) != 0)
pr_err("perf: amd_iommu: Only support raw events.\n");
 
+   perf_iommu->max_banks = amd_iommu_pc_get_max_banks(0);
+   perf_iommu->max_counters = amd_iommu_pc_get_max_counters(0);
+   if (!perf_iommu->max_banks || !perf_iommu->max_counters)
+   return -EINVAL;
+
/* Init null attributes */
perf_iommu->null_group = NULL;
perf_iommu->pmu.attr_groups = perf_iommu->attr_groups;
@@ -466,8 +463,8 @@ static __init int _init_perf_amd_iommu(
amd_iommu_pc_exit();
} else {
pr_info("perf: amd_iommu: Detected. (%d banks, %d 
counters/bank)\n",
-   amd_iommu_pc_get_max_banks(IOMMU_BASE_DEVID),
-   amd_iommu_pc_get_max_counters(IOMMU_BASE_DEVID));
+   amd_iommu_pc_get_max_banks(0),
+   amd_iommu_pc_get_max_counters(0));
}
 
return ret;
diff --git a/arch/x86/events/amd/iommu.h b/arch/x86/events/amd/iommu.h
index 845d173..0ce8c91 100644
--- a/arch/x86/events/amd/iommu.h
+++ b/arch/x86/events/amd/iommu.h
@@ -24,15 +24,12 @@
 #define PC_MAX_SPEC_BNKS   64
 #define PC_MAX_SPEC_CNTRS  16
 
-/* iommu pc reg masks*/
-#define IOMMU_BASE_DEVID   0x
-
 /* amd_iommu_init.c external support functions */
 extern bool amd_iommu_pc_supported(void);
 
-extern u8 amd_iommu_pc_get_max_banks(u16 devid);
+extern u8 amd_iommu_pc_get_max_banks(uint devid);
 
-extern u8 amd_iommu_pc_get_max_counters(u16 devid);
+extern u8 amd_iommu_pc_get_max_counters(uint devid);
 
 extern int amd_iommu_pc_get_set_reg_val(u16 devid, u8 bank, u8 cntr,
u8 fxn, u64 *value, bool is_write);
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 157e934..a7e756b 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -2706,6 +2706,19 @@ bool amd_iommu_v2_supported(void)
 }
 EXPORT_SYMBOL(amd_iommu_v2_supported);
 
+static struct amd_iommu *get_amd_iommu(uint idx)
+{
+   uint i = 0;
+   struct amd_iommu *iommu = NULL;
+
+   for_each_iommu(iommu) {
+   if (i == idx)
+   break;
+   i++;
+   }
+   return iommu;
+}
+
 /
  *
  * IOMMU EFR Performance Counter support functionality. This code allows
@@ -2713,17 +2726,14 @@ bool amd_iommu_v2_supported(void)
  *
  /
 
-u8 amd_iommu_pc_get_max_banks(u16 devid)
+u8 amd_iommu_pc_get_max_banks(uint idx)
 {
-   struct amd_iommu *iommu;
-   u8 ret = 0;
+   struct amd_iommu *iommu = get_amd_iommu(idx);
 
-   /* locate the iommu governing the devid */
-   iommu = amd_iommu_rlookup_table[devid];
if (iommu)
-   ret = iommu->max_banks;
+   return 

[PATCH V6 0/7] perf/amd/iommu: Enable multi-IOMMU support

2016-12-23 Thread Suravee Suthikulpanit
From: Suravee Suthikulpanit 

This patch series modifies the existing IOMMU and Perf drivers to support
systems with multiple IOMMUs by allocating an amd_iommu PMU per IOMMU instance.
This allows users to specify performance events and filters separately for each
IOMMU.

This has been tested on the new family17h-based server w/ multiple IOMMUs.

Git branch containing this patch series is available here:

https://github.com/ssuthiku/linux.git  perf-iommu-v6

Changes from V5 (https://lkml.org/lkml/2016/2/23/370)
  * Rebased onto v4.9.
  * Remove the patch which consolidates function delclarations since
we have not yet agreed on the appropriate place for the new header file.

Suravee Suthikulpanit (7):
  perf/amd/iommu: Misc fix up perf_iommu_read
  perf/amd/iommu: Modify functions to query max banks and counters
  perf/amd/iommu: Modify IOMMU API to allow specifying IOMMU index
  perf/amd/iommu: Declare pr_fmt and remove unnecessary pr_debug
  perf/amd/iommu: Clean up perf_iommu_enable_event
  iommu/amd: Introduce amd_iommu_get_num_iommus()
  perf/amd/iommu: Enable support for multiple IOMMUs

 arch/x86/events/amd/iommu.c | 206 
 arch/x86/events/amd/iommu.h |  17 ++--
 drivers/iommu/amd_iommu_init.c  | 119 ---
 drivers/iommu/amd_iommu_proto.h |  15 ++-
 4 files changed, 209 insertions(+), 148 deletions(-)

-- 
1.8.3.1

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


[PATCH V6 1/7] perf/amd/iommu: Misc fix up perf_iommu_read

2016-12-23 Thread Suravee Suthikulpanit
This patch contains the following minor fixup:
  * Fixed overflow handling since u64 delta would lose the MSB sign bit.
  * Remove unnecessary local64_set().
  * Coding style and make use of GENMASK_ULL macro.

Cc: Peter Zijlstra 
Cc: Borislav Petkov 
Signed-off-by: Suravee Suthikulpanit 
---
 arch/x86/events/amd/iommu.c | 25 +
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/arch/x86/events/amd/iommu.c b/arch/x86/events/amd/iommu.c
index b28200d..f387baf 100644
--- a/arch/x86/events/amd/iommu.c
+++ b/arch/x86/events/amd/iommu.c
@@ -319,29 +319,30 @@ static void perf_iommu_start(struct perf_event *event, 
int flags)
 
 static void perf_iommu_read(struct perf_event *event)
 {
-   u64 count = 0ULL;
-   u64 prev_raw_count = 0ULL;
-   u64 delta = 0ULL;
+   u64 cnt, prev;
+   s64 delta;
struct hw_perf_event *hwc = >hw;
pr_debug("perf: amd_iommu:perf_iommu_read\n");
 
amd_iommu_pc_get_set_reg_val(_GET_DEVID(event),
_GET_BANK(event), _GET_CNTR(event),
-   IOMMU_PC_COUNTER_REG, , false);
+   IOMMU_PC_COUNTER_REG, , false);
 
/* IOMMU pc counter register is only 48 bits */
-   count &= 0xULL;
+   cnt &= GENMASK_ULL(48, 0);
 
-   prev_raw_count =  local64_read(>prev_count);
-   if (local64_cmpxchg(>prev_count, prev_raw_count,
-   count) != prev_raw_count)
-   return;
+   prev = local64_read(>prev_count);
 
-   /* Handling 48-bit counter overflowing */
-   delta = (count << COUNTER_SHIFT) - (prev_raw_count << COUNTER_SHIFT);
+   /*
+* Since we do not enable counter overflow interrupts,
+* we do not have to worry about prev_count changing on us.
+*/
+   local64_set(>prev_count, cnt);
+
+   /* Handle 48-bit counter overflow */
+   delta = (cnt << COUNTER_SHIFT) - (prev << COUNTER_SHIFT);
delta >>= COUNTER_SHIFT;
local64_add(delta, >count);
-
 }
 
 static void perf_iommu_stop(struct perf_event *event, int flags)
-- 
1.8.3.1

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


Re: possible dmar_init_reserved_ranges() error

2016-12-23 Thread Joerg Roedel
On Thu, Dec 22, 2016 at 06:48:01PM -0600, Bjorn Helgaas wrote:
> If we didn't want to use pcibios_resource_to_bus() here for some
> reason, we should at least add a comment about why we think it's OK to
> use a CPU physical address as an IOVA.

Even if there are no such x86 systems out there, I think it doesn't hurt
to handle the possibility correctly in the IOMMU drivers.



Joerg

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