Re: [PATCH] dma-mapping: Use 'bitmap_zalloc()' when applicable
On Sun, 2021-10-24 at 19:40 +0200, Christophe JAILLET wrote: > 'dma_mem->bitmap' is a bitmap. So use 'bitmap_zalloc()' to simplify code, > improve the semantic and avoid some open-coded arithmetic in allocator > arguments. There is a cocci script for some of these. https://lore.kernel.org/all/08b89608cfb1280624d1a89ead6547069f9a4c31.ca...@perches.com/ ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v9 03/17] vdpa: Fix code indentation
On Tue, 2021-07-13 at 16:46 +0800, Xie Yongji wrote:
> Use tabs to indent the code instead of spaces.
There are a lot more of these in this file.
$ ./scripts/checkpatch.pl --fix-inplace --strict include/linux/vdpa.h
and a little typing gives:
---
include/linux/vdpa.h | 50 +-
1 file changed, 25 insertions(+), 25 deletions(-)
diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
index 3357ac98878d4..14cd4248e59fd 100644
--- a/include/linux/vdpa.h
+++ b/include/linux/vdpa.h
@@ -43,17 +43,17 @@ struct vdpa_vq_state_split {
* @last_used_idx: used index
*/
struct vdpa_vq_state_packed {
-u16last_avail_counter:1;
-u16last_avail_idx:15;
-u16last_used_counter:1;
-u16last_used_idx:15;
+ u16 last_avail_counter:1;
+ u16 last_avail_idx:15;
+ u16 last_used_counter:1;
+ u16 last_used_idx:15;
};
struct vdpa_vq_state {
- union {
- struct vdpa_vq_state_split split;
- struct vdpa_vq_state_packed packed;
- };
+ union {
+ struct vdpa_vq_state_split split;
+ struct vdpa_vq_state_packed packed;
+ };
};
struct vdpa_mgmt_dev;
@@ -131,7 +131,7 @@ struct vdpa_iova_range {
* @vdev: vdpa device
* @idx: virtqueue index
* @state: pointer to returned state
(last_avail_idx)
- * @get_vq_notification: Get the notification area for a virtqueue
+ * @get_vq_notification: Get the notification area for a virtqueue
* @vdev: vdpa device
* @idx: virtqueue index
* Returns the notifcation area
@@ -277,13 +277,13 @@ struct vdpa_device *__vdpa_alloc_device(struct device
*parent,
const struct vdpa_config_ops *config,
size_t size, const char *name);
-#define vdpa_alloc_device(dev_struct, member, parent, config, name) \
- container_of(__vdpa_alloc_device( \
- parent, config, \
- sizeof(dev_struct) + \
- BUILD_BUG_ON_ZERO(offsetof( \
- dev_struct, member)), name), \
- dev_struct, member)
+#define vdpa_alloc_device(dev_struct, member, parent, config, name)\
+ container_of(__vdpa_alloc_device(parent, config,\
+sizeof(dev_struct) + \
+BUILD_BUG_ON_ZERO(offsetof(dev_struct,
\
+ member)), \
+name), \
+dev_struct, member)
int vdpa_register_device(struct vdpa_device *vdev, int nvqs);
void vdpa_unregister_device(struct vdpa_device *vdev);
@@ -308,8 +308,8 @@ struct vdpa_driver {
int __vdpa_register_driver(struct vdpa_driver *drv, struct module *owner);
void vdpa_unregister_driver(struct vdpa_driver *drv);
-#define module_vdpa_driver(__vdpa_driver) \
- module_driver(__vdpa_driver, vdpa_register_driver, \
+#define module_vdpa_driver(__vdpa_driver) \
+ module_driver(__vdpa_driver, vdpa_register_driver, \
vdpa_unregister_driver)
static inline struct vdpa_driver *drv_to_vdpa(struct device_driver *driver)
@@ -339,25 +339,25 @@ static inline struct device *vdpa_get_dma_dev(struct
vdpa_device *vdev)
static inline void vdpa_reset(struct vdpa_device *vdev)
{
-const struct vdpa_config_ops *ops = vdev->config;
+ const struct vdpa_config_ops *ops = vdev->config;
vdev->features_valid = false;
-ops->set_status(vdev, 0);
+ ops->set_status(vdev, 0);
}
static inline int vdpa_set_features(struct vdpa_device *vdev, u64 features)
{
-const struct vdpa_config_ops *ops = vdev->config;
+ const struct vdpa_config_ops *ops = vdev->config;
vdev->features_valid = true;
-return ops->set_features(vdev, features);
+ return ops->set_features(vdev, features);
}
-
-static inline void vdpa_get_config(struct vdpa_device *vdev, unsigned offset,
+static inline void vdpa_get_config(struct vdpa_device *vdev,
+ unsigned int offset,
void *buf, unsigned int len)
{
-const struct vdpa_config_ops *ops = vdev->config;
+ const struct vdpa_config_ops *ops = vdev->config;
/*
* Config accesses aren't supposed to trigger before features are set.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/m
Re: [PATCH v2] iommu/amd: Fix extended features logging
On Mon, 2021-04-19 at 22:23 +0300, Alexander Monakov wrote:
> On Sun, 11 Apr 2021, Joe Perches wrote:
>
> > > v2: avoid pr_info(""), change pci_info() to pr_info() for a nicer
> > > solution
> > >
> > > drivers/iommu/amd/init.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> > > index 596d0c413473..62913f82a21f 100644
> > > --- a/drivers/iommu/amd/init.c
> > > +++ b/drivers/iommu/amd/init.c
> > > @@ -1929,8 +1929,8 @@ static void print_iommu_info(void)
> > > pci_info(pdev, "Found IOMMU cap 0x%hx\n", iommu->cap_ptr);
> > >
> > >
> > > if (iommu->cap & (1 << IOMMU_CAP_EFR)) {
> > > - pci_info(pdev, "Extended features (%#llx):",
> > > - iommu->features);
> > > + pr_info("Extended features (%#llx):", iommu->features);
> > > +
> > > for (i = 0; i < ARRAY_SIZE(feat_str); ++i) {
> > > if (iommu_feature(iommu, (1ULL << i)))
> > > pr_cont(" %s", feat_str[i]);
> >
> > How about avoiding all of this by using a temporary buffer
> > and a single pci_info.
>
> I think it is mostly up to the maintainers, but from my perspective, it's not
> good to conflate such a simple bugfix with the substantial rewrite you are
> proposing (which also increases code complexity).
You and I have _significant_ differences in the definition of substantial.
Buffering the output is the preferred code style in preference to
pr_cont.
Do remember pr_cont should _only_ be used when absolutely necessary
as interleaving of other messages from other processes/threads can
and does occur.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/amd: Fix extended features logging
On Mon, 2021-04-12 at 00:13 +0300, Alexander Monakov wrote:
> print_iommu_info prints the EFR register and then the decoded list of
> features on a separate line:
>
> pci :00:00.2: AMD-Vi: Extended features (0x206d73ef22254ade):
> PPR X2APIC NX GT IA GA PC GA_vAPIC
>
> The second line is emitted via 'pr_cont', which causes it to have a
> different ('warn') loglevel compared to the previous line ('info').
>
> Commit 9a295ff0ffc9 attempted to rectify this by removing the newline
> from the pci_info format string, but this doesn't work, as pci_info
> calls implicitly append a newline anyway.
>
> Printing the decoded features on the same line would make it quite long.
> Instead, change pci_info() to pr_info() to omit PCI bus location info,
> which is shown in the preceding message anyway. This results in:
>
> pci :00:00.2: AMD-Vi: Found IOMMU cap 0x40
> AMD-Vi: Extended features (0x206d73ef22254ade): PPR X2APIC NX GT IA GA PC
> GA_vAPIC
> AMD-Vi: Interrupt remapping enabled
>
> Fixes: 9a295ff0ffc9 ("iommu/amd: Print extended features in one line to fix
> divergent log levels")
> Link:
> https://lore.kernel.org/lkml/alpine.lnx.2.20.13.2104112326460.11...@monopod.intra.ispras.ru
> Signed-off-by: Alexander Monakov
> Cc: Paul Menzel
> Cc: Joerg Roedel
> Cc: Suravee Suthikulpanit
> Cc: iommu@lists.linux-foundation.org
> ---
>
> v2: avoid pr_info(""), change pci_info() to pr_info() for a nicer
> solution
>
> drivers/iommu/amd/init.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> index 596d0c413473..62913f82a21f 100644
> --- a/drivers/iommu/amd/init.c
> +++ b/drivers/iommu/amd/init.c
> @@ -1929,8 +1929,8 @@ static void print_iommu_info(void)
> pci_info(pdev, "Found IOMMU cap 0x%hx\n", iommu->cap_ptr);
>
>
> if (iommu->cap & (1 << IOMMU_CAP_EFR)) {
> - pci_info(pdev, "Extended features (%#llx):",
> - iommu->features);
> + pr_info("Extended features (%#llx):", iommu->features);
> +
> for (i = 0; i < ARRAY_SIZE(feat_str); ++i) {
> if (iommu_feature(iommu, (1ULL << i)))
> pr_cont(" %s", feat_str[i]);
How about avoiding all of this by using a temporary buffer
and a single pci_info.
Miscellanea:
o Move the feat_str and i declarations into the if block for locality
---
drivers/iommu/amd/init.c | 29 ++---
1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index 321f5906e6ed..0d219044726e 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -1943,30 +1943,37 @@ static int __init iommu_init_pci(struct amd_iommu
*iommu)
static void print_iommu_info(void)
{
- static const char * const feat_str[] = {
- "PreF", "PPR", "X2APIC", "NX", "GT", "[5]",
- "IA", "GA", "HE", "PC"
- };
struct amd_iommu *iommu;
for_each_iommu(iommu) {
struct pci_dev *pdev = iommu->dev;
- int i;
pci_info(pdev, "Found IOMMU cap 0x%x\n", iommu->cap_ptr);
if (iommu->cap & (1 << IOMMU_CAP_EFR)) {
- pci_info(pdev, "Extended features (%#llx):",
-iommu->features);
+ static const char * const feat_str[] = {
+ "PreF", "PPR", "X2APIC", "NX", "GT", "[5]",
+ "IA", "GA", "HE", "PC"
+ };
+ int i;
+ char features[128] = "";
+ int len = 0;
+
for (i = 0; i < ARRAY_SIZE(feat_str); ++i) {
- if (iommu_feature(iommu, (1ULL << i)))
- pr_cont(" %s", feat_str[i]);
+ if (!iommu_feature(iommu, BIT_ULL(i)))
+ continue;
+ len += scnprintf(features + len,
+sizeof(features) - len,
+" %s", feat_str[i]);
}
if (iommu->features & FEATURE_GAM_VAPIC)
- pr_cont(" GA_vAPIC");
+ len += scnprintf(features + len,
+sizeof(features) - len,
+" %s", "GA_vPIC");
- pr_cont("\n");
+ pci_info(pdev, "Extended features (%#llx):%s\n",
+iommu->features, features);
}
}
if (irq_remapping_enabled) {
___
iommu mailing list
iommu
Re: [PATCH] iommu/amd: Fix extended features logging
On Sun, 2021-04-11 at 21:52 +0200, John Ogness wrote: > I'd rather fix dev_info callers to allow pr_cont and then fix any code > that is using this workaround. Assuming you mean all dev_() uses, me too. > And if the print maintainers agree it is OK to encourage > pr_cont(LOGLEVEL "...") usage, then people should really start using > that if the loglevel on those pieces is important. I have no stong feeling about the use of pr_cont( as valuable or not. I think it's just a trivial bit that could be somewhat useful when interleaving occurs. A somewhat better mechanism would be to have an explicit cookie use like: cookie = printk_multipart_init(KERN_LEVEL, fmt, ...); while () printk_multipart_cont(cookie, fmt, ...); printk_multipark_end(cookie, fmt, ...); And separately, there should be a pr_debug_cont or equivalent. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/amd: Fix extended features logging
(cc'ing the printk maintainers)
On Sun, 2021-04-11 at 00:11 +0300, Alexander Monakov wrote:
> print_iommu_info prints the EFR register and then the decoded list of
> features on a separate line:
>
> pci :00:00.2: AMD-Vi: Extended features (0x206d73ef22254ade):
> PPR X2APIC NX GT IA GA PC GA_vAPIC
>
> The second line is emitted via 'pr_cont', which causes it to have a
> different ('warn') loglevel compared to the previous line ('info').
>
> Commit 9a295ff0ffc9 attempted to rectify this by removing the newline
> from the pci_info format string, but this doesn't work, as pci_info
> calls implicitly append a newline anyway.
>
> Restore the newline, and call pr_info with empty format string to set
> the loglevel for subsequent pr_cont calls. The same solution is used in
> EFI and uvesafb drivers.
>
> Fixes: 9a295ff0ffc9 ("iommu/amd: Print extended features in one line to fix
> divergent log levels")
> Signed-off-by: Alexander Monakov
> Cc: Paul Menzel
> Cc: Joerg Roedel
> Cc: Suravee Suthikulpanit
> Cc: iommu@lists.linux-foundation.org
> ---
> drivers/iommu/amd/init.c | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> index 596d0c413473..a25e241eff1c 100644
> --- a/drivers/iommu/amd/init.c
> +++ b/drivers/iommu/amd/init.c
> @@ -1929,8 +1929,11 @@ static void print_iommu_info(void)
> pci_info(pdev, "Found IOMMU cap 0x%hx\n", iommu->cap_ptr);
>
>
> if (iommu->cap & (1 << IOMMU_CAP_EFR)) {
> - pci_info(pdev, "Extended features (%#llx):",
> + pci_info(pdev, "Extended features (%#llx):\n",
> iommu->features);
> +
> + pr_info("");
> +
> for (i = 0; i < ARRAY_SIZE(feat_str); ++i) {
> if (iommu_feature(iommu, (1ULL << i)))
> pr_cont(" %s", feat_str[i]);
This shouldn't be necessary.
If this is true then a lot of output logging code broke.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [trivial PATCH] treewide: Convert switch/case fallthrough; to break;
On Thu, 2020-09-10 at 15:21 +0100, Robin Murphy wrote:
> On 2020-09-09 21:06, Joe Perches wrote:
> > fallthrough to a separate case/default label break; isn't very readable.
> >
> > Convert pseudo-keyword fallthrough; statements to a simple break; when
> > the next label is case or default and the only statement in the next
> > label block is break;
> >
> > Found using:
> >
> > $ grep-2.5.4 -rP --include=*.[ch] -n
> > "fallthrough;(\s*(case\s+\w+|default)\s*:\s*){1,7}break;" *
> >
> > Miscellanea:
> >
> > o Move or coalesce a couple label blocks above a default: block.
> >
> > Signed-off-by: Joe Perches
> > ---
> >
> > Compiled allyesconfig x86-64 only.
> > A few files for other arches were not compiled.
> >
>
> [...]
> > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > index c192544e874b..743db1abec40 100644
> > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > @@ -3777,7 +3777,7 @@ static int arm_smmu_device_hw_probe(struct
> > arm_smmu_device *smmu)
> > switch (FIELD_GET(IDR0_TTF, reg)) {
> > case IDR0_TTF_AARCH32_64:
> > smmu->ias = 40;
> > - fallthrough;
> > + break;
> > case IDR0_TTF_AARCH64:
> > break;
> > default:
>
> I have to say I don't really agree with the readability argument for
> this one - a fallthrough is semantically correct here, since the first
> case is a superset of the second. It just happens that anything we would
> do for the common subset is implicitly assumed (there are other
> potential cases we simply haven't added support for at the moment), thus
> the second case is currently empty.
> This change actively obfuscates that distinction.
Then perhaps comments should be added to usefully
describe the mechanisms.
case IDR0_TTF_AARCH32_64:
smmu->ias = 40;
fallthrough;/* and still do the 64 bit processing */
case IDR0_TTF_AARCH64:
/* Nothing specific yet */
break;
> Robin.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [trivial PATCH] treewide: Convert switch/case fallthrough; to break;
On Wed, 2020-09-09 at 19:36 -0300, Jason Gunthorpe wrote:
> On Wed, Sep 09, 2020 at 01:06:39PM -0700, Joe Perches wrote:
> > fallthrough to a separate case/default label break; isn't very readable.
> >
> > Convert pseudo-keyword fallthrough; statements to a simple break; when
> > the next label is case or default and the only statement in the next
> > label block is break;
> >
> > Found using:
> >
> > $ grep-2.5.4 -rP --include=*.[ch] -n
> > "fallthrough;(\s*(case\s+\w+|default)\s*:\s*){1,7}break;" *
> >
> > Miscellanea:
> >
> > o Move or coalesce a couple label blocks above a default: block.
> >
> > Signed-off-by: Joe Perches
> > ---
> >
> > Compiled allyesconfig x86-64 only.
> > A few files for other arches were not compiled.
>
> IB part looks OK, I prefer it like this
>
> You could do the same for continue as well, I saw a few of those..
I saw some continue uses as well but wasn't sure
and didn't look to see if the switch/case with
continue was in a for/while loop.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[trivial PATCH] treewide: Convert switch/case fallthrough; to break;
fallthrough to a separate case/default label break; isn't very readable.
Convert pseudo-keyword fallthrough; statements to a simple break; when
the next label is case or default and the only statement in the next
label block is break;
Found using:
$ grep-2.5.4 -rP --include=*.[ch] -n
"fallthrough;(\s*(case\s+\w+|default)\s*:\s*){1,7}break;" *
Miscellanea:
o Move or coalesce a couple label blocks above a default: block.
Signed-off-by: Joe Perches
---
Compiled allyesconfig x86-64 only.
A few files for other arches were not compiled.
arch/arm/mach-mmp/pm-pxa910.c | 2 +-
arch/arm64/kvm/handle_exit.c | 2 +-
arch/mips/kernel/cpu-probe.c | 2 +-
arch/mips/math-emu/cp1emu.c | 2 +-
arch/s390/pci/pci.c | 2 +-
crypto/tcrypt.c | 4 ++--
drivers/ata/sata_mv.c | 2 +-
drivers/atm/lanai.c | 2 +-
drivers/gpu/drm/i915/display/intel_sprite.c | 2 +-
drivers/gpu/drm/nouveau/nvkm/engine/disp/hdmi.c | 2 +-
drivers/hid/wacom_wac.c | 2 +-
drivers/i2c/busses/i2c-i801.c | 2 +-
drivers/infiniband/ulp/rtrs/rtrs-clt.c| 14 +++---
drivers/infiniband/ulp/rtrs/rtrs-srv.c| 6 +++---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 2 +-
drivers/irqchip/irq-vic.c | 4 ++--
drivers/md/dm.c | 2 +-
drivers/media/dvb-frontends/drxd_hard.c | 2 +-
drivers/media/i2c/ov5640.c| 2 +-
drivers/media/i2c/ov6650.c| 5 ++---
drivers/media/i2c/smiapp/smiapp-core.c| 2 +-
drivers/media/i2c/tvp5150.c | 2 +-
drivers/media/pci/ddbridge/ddbridge-core.c| 2 +-
drivers/media/usb/cpia2/cpia2_core.c | 2 +-
drivers/mfd/iqs62x.c | 3 +--
drivers/mmc/host/atmel-mci.c | 2 +-
drivers/mtd/nand/raw/nandsim.c| 2 +-
drivers/net/ethernet/intel/e1000e/phy.c | 2 +-
drivers/net/ethernet/intel/fm10k/fm10k_pf.c | 2 +-
drivers/net/ethernet/intel/i40e/i40e_adminq.c | 2 +-
drivers/net/ethernet/intel/i40e/i40e_txrx.c | 2 +-
drivers/net/ethernet/intel/iavf/iavf_txrx.c | 2 +-
drivers/net/ethernet/intel/igb/e1000_phy.c| 2 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_82599.c| 2 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c| 2 +-
drivers/net/ethernet/intel/ixgbevf/vf.c | 2 +-
drivers/net/ethernet/netronome/nfp/nfpcore/nfp6000_pcie.c | 2 +-
drivers/net/ethernet/qlogic/qed/qed_mcp.c | 2 +-
drivers/net/ethernet/sfc/falcon/farch.c | 2 +-
drivers/net/ethernet/sfc/farch.c | 2 +-
drivers/net/phy/adin.c| 3 +--
drivers/net/usb/pegasus.c | 4 ++--
drivers/net/usb/usbnet.c | 2 +-
drivers/net/wireless/ath/ath5k/eeprom.c | 2 +-
drivers/net/wireless/mediatek/mt7601u/dma.c | 8
drivers/nvme/host/core.c | 12 ++--
drivers/pcmcia/db1xxx_ss.c| 4 ++--
drivers/power/supply/abx500_chargalg.c| 2 +-
drivers/power/supply/charger-manager.c| 2 +-
drivers/rtc/rtc-pcf85063.c| 2 +-
drivers/s390/scsi/zfcp_fsf.c | 2 +-
drivers/scsi/aic7xxx/aic79xx_core.c | 4 ++--
drivers/scsi/aic94xx/aic94xx_tmf.c| 2 +-
drivers/scsi/lpfc/lpfc_sli.c | 2 +-
drivers/scsi/smartpqi/smartpqi_init.c | 2 +-
drivers/scsi/sr.c | 2 +-
drivers/tty/serial/sunsu.c| 2 +-
drivers/tty/serial/sunzilog.c | 2 +-
drivers/tty/vt/vt_ioctl.c | 2 +-
drivers/usb/dwc3/core.c | 2 +-
drivers/usb/gadget/legacy/inode.c | 2 +-
drivers/usb/gadget/udc/pxa25x_udc.c | 4 ++--
drivers/usb/host/ohci-hcd.c | 2 +-
drivers/usb/isp1760/isp1760-hcd.c | 2 +-
drivers/usb/
[Possible PATCH] iommu/qcom: Change CONFIG_BIG_ENDIAN to CONFIG_CPU_BIG_ENDIAN
CONFIG_BIG_ENDIAN does not exist as a Kconfig symbol. Signed-off-by: Joe Perches --- I don't have the hardware, so I can't tell if this is a correct change, but it is a logical one. Found by a test script that looks for IS_ENABLED(FOO) where FOO must also exist in Kconfig files. drivers/iommu/qcom_iommu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iommu/qcom_iommu.c b/drivers/iommu/qcom_iommu.c index c3e1fbd1988c..69e113471ecb 100644 --- a/drivers/iommu/qcom_iommu.c +++ b/drivers/iommu/qcom_iommu.c @@ -304,7 +304,7 @@ static int qcom_iommu_init_domain(struct iommu_domain *domain, ARM_SMMU_SCTLR_M | ARM_SMMU_SCTLR_S1_ASIDPNE | ARM_SMMU_SCTLR_CFCFG; - if (IS_ENABLED(CONFIG_BIG_ENDIAN)) + if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN)) reg |= ARM_SMMU_SCTLR_E; iommu_writel(ctx, ARM_SMMU_CB_SCTLR, reg); ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 5.6 61/73] iommu/vt-d: Use right Kconfig option name
On Mon, 2020-05-04 at 19:58 +0200, Greg Kroah-Hartman wrote:
> From: Lu Baolu
>
> commit ba61c3da00f4a5bf8805aeca1ba5ac3c9bd82e96 upstream.
>
> The CONFIG_ prefix should be added in the code.
>
> Fixes: 046182525db61 ("iommu/vt-d: Add Kconfig option to enable/disable
> scalable mode")
> Reported-and-tested-by: Kumar, Sanjay K
> Signed-off-by: Lu Baolu
> Cc: Ashok Raj
> Link:
> https://lore.kernel.org/r/20200501072427.14265-1-baolu...@linux.intel.com
> Signed-off-by: Joerg Roedel
> Signed-off-by: Greg Kroah-Hartman
>
> ---
> drivers/iommu/intel-iommu.c |4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -371,11 +371,11 @@ int dmar_disabled = 0;
> int dmar_disabled = 1;
> #endif /* CONFIG_INTEL_IOMMU_DEFAULT_ON */
>
> -#ifdef INTEL_IOMMU_SCALABLE_MODE_DEFAULT_ON
> +#ifdef CONFIG_INTEL_IOMMU_SCALABLE_MODE_DEFAULT_ON
> int intel_iommu_sm = 1;
> #else
> int intel_iommu_sm;
> -#endif /* INTEL_IOMMU_SCALABLE_MODE_DEFAULT_ON */
> +#endif /* CONFIG_INTEL_IOMMU_SCALABLE_MODE_DEFAULT_ON */
Perhaps simpler as
int intel_iommu_sm = IS_BUILTIN(CONFIG_INTEL_IOMMU_SCALABLE_MODE_DEFAULT_ON);
So perhaps:
---
drivers/iommu/intel-iommu.c | 13 ++---
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 0182cff2c7ac..ab8552c48391 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -365,17 +365,8 @@ static int intel_iommu_attach_device(struct iommu_domain
*domain,
static phys_addr_t intel_iommu_iova_to_phys(struct iommu_domain *domain,
dma_addr_t iova);
-#ifdef CONFIG_INTEL_IOMMU_DEFAULT_ON
-int dmar_disabled = 0;
-#else
-int dmar_disabled = 1;
-#endif /* CONFIG_INTEL_IOMMU_DEFAULT_ON */
-
-#ifdef CONFIG_INTEL_IOMMU_SCALABLE_MODE_DEFAULT_ON
-int intel_iommu_sm = 1;
-#else
-int intel_iommu_sm;
-#endif /* CONFIG_INTEL_IOMMU_SCALABLE_MODE_DEFAULT_ON */
+int dmar_disabled = !IS_BUILTIN(CONFIG_INTEL_IOMMU_DEFAULT_ON);
+int intel_iommu_sm = IS_BUILTIN(CONFIG_INTEL_IOMMU_SCALABLE_MODE_DEFAULT_ON);
int intel_iommu_enabled = 0;
EXPORT_SYMBOL_GPL(intel_iommu_enabled);
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] treewide: Rename "unencrypted" to "decrypted"
On Tue, 2020-03-17 at 14:24 -0700, Dave Hansen wrote: > On 3/17/20 2:06 PM, Borislav Petkov wrote: > > On Tue, Mar 17, 2020 at 01:35:12PM -0700, Dave Hansen wrote: > > > On 3/17/20 4:18 AM, Borislav Petkov wrote: > > > > Back then when the whole SME machinery started getting mainlined, it > > > > was agreed that for simplicity, clarity and sanity's sake, the terms > > > > denoting encrypted and not-encrypted memory should be "encrypted" and > > > > "decrypted". And the majority of the code sticks to that convention > > > > except those two. So rename them. > > > Don't "unencrypted" and "decrypted" mean different things? > > > > > > Unencrypted to me means "encryption was never used for this data". > > > > > > Decrypted means "this was/is encrypted but here is a plaintext copy". > > Maybe but linguistical semantics is not the point here. > > > > The idea is to represent a "binary" concept of memory being encrypted > > or memory being not encrypted. And at the time we decided to use > > "encrypted" and "decrypted" for those two things. > > Yeah, agreed. We're basically trying to name "!encrypted". > > > Do you see the need to differentiate a third "state", so to speak, of > > memory which was never encrypted? > > No, there are just two states. I just think the "!encrypted" case > should not be called "decrypted". Nor do I, it's completely misleading. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH -next 000/491] treewide: use fallthrough;
There is a new fallthrough pseudo-keyword macro that can be used
to replace the various /* fallthrough */ style comments that are
used to indicate a case label code block is intended to fallthrough
to the next case label block.
See commit 294f69e662d1 ("compiler_attributes.h: Add 'fallthrough'
pseudo keyword for switch/case use")
These patches are intended to allow clang to detect missing
switch/case fallthrough uses.
Do a depth-first pass on the MAINTAINERS file and find the various
F: pattern files and convert the fallthrough comments to fallthrough;
for all files matched by all F: patterns in in each section.
Done via the perl script below and the previously posted
cvt_fallthrough.pl script.
Link:
https://lore.kernel.org/lkml/b56602fcf79f849e733e7b521bb0e17895d390fa.1582230379.git.joe.com/
These patches are based on next-20200310 and are available in
git://repo.or.cz/linux-2.6/trivial-mods.git in branch 20200310_fallthrough_2
$ cat commit_fallthrough.pl
#!/usr/bin/env perl
use sort 'stable';
#
# Reorder a sorted array so file entries are before directory entries
# depends on a trailing / for directories
# so:
# foo/
# foo/bar.c
# becomes
# foo/bar.c
# foo/
#
sub file_before_directory {
my ($array_ref) = (@_);
my $count = scalar(@$array_ref);
for (my $i = 1; $i < $count; $i++) {
if (substr(@$array_ref[$i - 1], -1) eq '/' &&
substr(@$array_ref[$i], 0, length(@$array_ref[$i - 1])) eq
@$array_ref[$i - 1]) {
my $string = @$array_ref[$i - 1];
@$array_ref[$i - 1] = @$array_ref[$i];
@$array_ref[$i] = $string;
}
}
}
sub uniq {
my (@parms) = @_;
my %saw;
@parms = grep(!$saw{$_}++, @parms);
return @parms;
}
# Get all the F: file patterns in MAINTAINERS that could be a .[ch] file
my $maintainer_patterns = `grep -P '^F:\\s+' MAINTAINERS`;
my @patterns = split('\n', $maintainer_patterns);
s/^F:\s*// for @patterns;
@patterns = grep(!/^(?:Documentation|tools|scripts)\//, @patterns);
@patterns = grep(!/\.(?:dtsi?|rst|config)$/, @patterns);
@patterns = sort @patterns;
@patterns = sort { $b =~ tr/\//\// cmp $a =~ tr/\//\// } @patterns;
file_before_directory(\@patterns);
my %sections_done;
foreach my $pattern (@patterns) {
# Find the files the pattern matches
my $pattern_files = `git ls-files -- $pattern`;
my @new_patterns = split('\n', $pattern_files);
$pattern_files = join(' ', @new_patterns);
next if ($pattern_files =~ /^\s*$/);
# Find the section the first file matches
my $pattern_file = @new_patterns[0];
my $section_output = `./scripts/get_maintainer.pl --nogit --nogit-fallback
--sections --pattern-depth=1 $pattern_file`;
my @section = split('\n', $section_output);
my $section_header = @section[0];
print("Section: <$section_header>\n");
# Skip the section if it's already done
print("Already done '$section_header'\n") if
($sections_done{$section_header});
next if ($sections_done{$section_header}++);
# Find all the .[ch] files in all F: lines in that section
my @new_section;
foreach my $line (@section) {
last if ($line =~ /^\s*$/);
push(@new_section, $line);
}
@section = grep(/^F:/, @new_section);
s/^F:\s*// for @section;
@section = grep(!/^(?:Documentation|tools|scripts)\//, @section);
@section = grep(!/\.(?:dtsi?|rst|config)$/, @section);
@section = sort @section;
@section = uniq(@section);
my $section_files = join(' ', @section);
print("section_files: <$section_files>\n");
next if ($section_files =~ /^\s*$/);
my $cvt_files = `git ls-files -- $section_files`;
my @files = split('\n', $cvt_files);
@files = grep(!/^(?:Documentation|tools|scripts)\//, @files);
@files = grep(!/\.(?:dtsi?|rst|config)$/, @files);
@files = grep(/\.[ch]$/, @files);
@files = sort @files;
@files = uniq(@files);
$cvt_files = join(' ', @files);
print("files: <$cvt_files>\n");
next if (scalar(@files) < 1);
# Convert fallthroughs for all [.ch] files in the section
print("doing cvt_fallthrough.pl -- $cvt_files\n");
`cvt_fallthrough.pl -- $cvt_files`;
# If nothing changed, nothing to commit
`git diff-index --quiet HEAD --`;
next if (!$?);
# Commit the changes
my $fh;
open($fh, "+>", "cvt_fallthrough.commit_msg") or die "$0: can't create
temporary file: $!\n";
print $fh <https://lore.kernel.org/lkml/b56602fcf79f849e733e7b521bb0e17895d390fa.1582230379.git@perches.com/
EOF
;
close $fh;
`git commit -s -a -F cvt_fallthrough.commit_msg`;
}
Joe Perches (491):
MELLANOX ETHERNET INNOVA DRIVERS: Use fallthrough;
MARVELL OCTEONTX2 RVU ADMIN FUNCTION DRI
Re: [PATCH v5 8/8] iommu/vt-d: Misc macro clean up for SVM
On Mon, 2019-12-02 at 11:58 -0800, Jacob Pan wrote:
> Use combined macros for_each_svm_dev() to simplify SVM device iteration
> and error checking.
[]
> diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
[]
> @@ -427,40 +430,36 @@ int intel_svm_unbind_mm(struct device *dev, int pasid)
[]
> + for_each_svm_dev(sdev, svm, dev) {
> + ret = 0;
> + sdev->users--;
> + if (!sdev->users) {
This might be better by reducing indentation here too
if (sdev->users)
break;
> + list_del_rcu(&sdev->list);
to reduce indentation 1 level below this
> + /* Flush the PASID cache and IOTLB for this device.
> + * Note that we do depend on the hardware *not* using
> + * the PASID any more. Just as we depend on other
> + * devices never using PASIDs that they have no right
> + * to use. We have a *shared* PASID table, because it's
> + * large and has to be physically contiguous. So it's
> + * hard to be as defensive as we might like. */
> + intel_pasid_tear_down_entry(iommu, dev, svm->pasid);
> + intel_flush_svm_range_dev(svm, sdev, 0, -1, 0);
> + kfree_rcu(sdev, rcu);
> +
> + if (list_empty(&svm->devs)) {
> + ioasid_free(svm->pasid);
> + if (svm->mm)
> + mmu_notifier_unregister(&svm->notifier,
> svm->mm);
> + list_del(&svm->list);
> + /* We mandate that no page faults may be
> outstanding
> + * for the PASID when intel_svm_unbind_mm() is
> called.
> + * If that is not obeyed, subtle errors will
> happen.
> + * Let's make them less subtle... */
> + memset(svm, 0x6b, sizeof(*svm));
> + kfree(svm);
> }
> - break;
> }
> + break;
> }
> out:
> mutex_unlock(&pasid_mutex);
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 8/8] iommu/vt-d: Misc macro clean up for SVM
On Mon, 2019-12-02 at 10:15 -0800, Jacob Pan wrote:
> On Thu, 21 Nov 2019 13:37:10 -0800
> Joe Perches wrote:
>
> > On Thu, 2019-11-21 at 13:26 -0800, Jacob Pan wrote:
> > > Use combined macros for_each_svm_dev() to simplify SVM device
> > > iteration and error checking.
> > []
> > > diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
> > []
> > > +#define for_each_svm_dev(sdev, svm, d) \
> > > + list_for_each_entry((sdev), &(svm)->devs, list) \
> > > + if ((d) != (sdev)->dev) {} else
> > > +
> > > int intel_svm_bind_mm(struct device *dev, int *pasid, int flags,
> > > struct svm_dev_ops *ops) {
> > > struct intel_iommu *iommu = intel_svm_device_to_iommu(dev);
> > > @@ -274,15 +278,13 @@ int intel_svm_bind_mm(struct device *dev, int
> > > *pasid, int flags, struct svm_dev_ goto out;
> > > }
> > >
> > > - list_for_each_entry(sdev, &svm->devs,
> > > list) {
> > > - if (dev == sdev->dev) {
> > > - if (sdev->ops != ops) {
> > > - ret = -EBUSY;
> > > - goto out;
> > > - }
> > > - sdev->users++;
> > > - goto success;
> > > + for_each_svm_dev(sdev, svm, dev) {
> > > + if (sdev->ops != ops) {
> > > + ret = -EBUSY;
> > > + goto out;
> > > }
> > > + sdev->users++;
> > > + goto success;
> > > }
> >
> > I think this does not read better as this is now a
> > for_each loop that exits the loop on the first match.
> >
> I think one of the benefits is reduced indentation. What do you
> recommend?
Making the code intelligible for a reader.
At least add a comment describing why there is only
a single possible match.
Given the for_each name, it's odd code that only the
first match has an action.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/iova: silence warnings under memory pressure
On Fri, 2019-11-22 at 11:46 -0500, Qian Cai wrote:
> On Fri, 2019-11-22 at 08:28 -0800, Joe Perches wrote:
> > On Fri, 2019-11-22 at 09:59 -0500, Qian Cai wrote:
> > > On Thu, 2019-11-21 at 20:37 -0800, Joe Perches wrote:
> > > > On Thu, 2019-11-21 at 21:55 -0500, Qian Cai wrote:
> > > > > When running heavy memory pressure workloads, this 5+ old system is
> > > > > throwing endless warnings below because disk IO is too slow to recover
> > > > > from swapping. Since the volume from alloc_iova_fast() could be large,
> > > > > once it calls printk(), it will trigger disk IO (writing to the log
> > > > > files) and pending softirqs which could cause an infinite loop and
> > > > > make
> > > > > no progress for days by the ongoimng memory reclaim. This is the
> > > > > counter
> > > > > part for Intel where the AMD part has already been merged. See the
> > > > > commit 3d708895325b ("iommu/amd: Silence warnings under memory
> > > > > pressure"). Since the allocation failure will be reported in
> > > > > intel_alloc_iova(), so just call printk_ratelimted() there and silence
> > > > > the one in alloc_iova_mem() to avoid the expensive warn_alloc().
> > > >
> > > > []
> > > > > v2: use dev_err_ratelimited() and improve the commit messages.
> > > >
> > > > []
> > > > > diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> > > >
> > > > []
> > > > > @@ -3401,7 +3401,8 @@ static unsigned long intel_alloc_iova(struct
> > > > > device *dev,
> > > > > iova_pfn = alloc_iova_fast(&domain->iovad, nrpages,
> > > > > IOVA_PFN(dma_mask), true);
> > > > > if (unlikely(!iova_pfn)) {
> > > > > - dev_err(dev, "Allocating %ld-page iova failed",
> > > > > nrpages);
> > > > > + dev_err_ratelimited(dev, "Allocating %ld-page iova
> > > > > failed",
> > > > > + nrpages);
> > > >
> > > > Trivia:
> > > >
> > > > This should really have a \n termination on the format string
> > > >
> > > > dev_err_ratelimited(dev, "Allocating %ld-page iova
> > > > failed\n",
> > > >
> > > >
> > >
> > > Why do you say so? It is right now printing with a newline added anyway.
> > >
> > > hpsa :03:00.0: DMAR: Allocating 1-page iova failed
> >
> > If another process uses pr_cont at the same time,
> > it can be interleaved.
>
> I lean towards fixing that in a separate patch if ever needed, as the origin
> dev_err() has no "\n" enclosed either.
Your choice.
I wrote trivia:, but touching the same line multiple times
is relatively pointless.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/iova: silence warnings under memory pressure
On Fri, 2019-11-22 at 09:59 -0500, Qian Cai wrote:
> On Thu, 2019-11-21 at 20:37 -0800, Joe Perches wrote:
> > On Thu, 2019-11-21 at 21:55 -0500, Qian Cai wrote:
> > > When running heavy memory pressure workloads, this 5+ old system is
> > > throwing endless warnings below because disk IO is too slow to recover
> > > from swapping. Since the volume from alloc_iova_fast() could be large,
> > > once it calls printk(), it will trigger disk IO (writing to the log
> > > files) and pending softirqs which could cause an infinite loop and make
> > > no progress for days by the ongoimng memory reclaim. This is the counter
> > > part for Intel where the AMD part has already been merged. See the
> > > commit 3d708895325b ("iommu/amd: Silence warnings under memory
> > > pressure"). Since the allocation failure will be reported in
> > > intel_alloc_iova(), so just call printk_ratelimted() there and silence
> > > the one in alloc_iova_mem() to avoid the expensive warn_alloc().
> >
> > []
> > > v2: use dev_err_ratelimited() and improve the commit messages.
> >
> > []
> > > diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> >
> > []
> > > @@ -3401,7 +3401,8 @@ static unsigned long intel_alloc_iova(struct device
> > > *dev,
> > > iova_pfn = alloc_iova_fast(&domain->iovad, nrpages,
> > > IOVA_PFN(dma_mask), true);
> > > if (unlikely(!iova_pfn)) {
> > > - dev_err(dev, "Allocating %ld-page iova failed", nrpages);
> > > + dev_err_ratelimited(dev, "Allocating %ld-page iova failed",
> > > + nrpages);
> >
> > Trivia:
> >
> > This should really have a \n termination on the format string
> >
> > dev_err_ratelimited(dev, "Allocating %ld-page iova failed\n",
> >
> >
>
> Why do you say so? It is right now printing with a newline added anyway.
>
> hpsa :03:00.0: DMAR: Allocating 1-page iova failed
If another process uses pr_cont at the same time,
it can be interleaved.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/iova: silence warnings under memory pressure
On Thu, 2019-11-21 at 21:55 -0500, Qian Cai wrote:
> When running heavy memory pressure workloads, this 5+ old system is
> throwing endless warnings below because disk IO is too slow to recover
> from swapping. Since the volume from alloc_iova_fast() could be large,
> once it calls printk(), it will trigger disk IO (writing to the log
> files) and pending softirqs which could cause an infinite loop and make
> no progress for days by the ongoimng memory reclaim. This is the counter
> part for Intel where the AMD part has already been merged. See the
> commit 3d708895325b ("iommu/amd: Silence warnings under memory
> pressure"). Since the allocation failure will be reported in
> intel_alloc_iova(), so just call printk_ratelimted() there and silence
> the one in alloc_iova_mem() to avoid the expensive warn_alloc().
[]
> v2: use dev_err_ratelimited() and improve the commit messages.
[]
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
[]
> @@ -3401,7 +3401,8 @@ static unsigned long intel_alloc_iova(struct device
> *dev,
> iova_pfn = alloc_iova_fast(&domain->iovad, nrpages,
> IOVA_PFN(dma_mask), true);
> if (unlikely(!iova_pfn)) {
> - dev_err(dev, "Allocating %ld-page iova failed", nrpages);
> + dev_err_ratelimited(dev, "Allocating %ld-page iova failed",
> + nrpages);
Trivia:
This should really have a \n termination on the format string
dev_err_ratelimited(dev, "Allocating %ld-page iova failed\n",
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 8/8] iommu/vt-d: Misc macro clean up for SVM
On Thu, 2019-11-21 at 13:26 -0800, Jacob Pan wrote:
> Use combined macros for_each_svm_dev() to simplify SVM device iteration
> and error checking.
[]
> diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
[]
> +#define for_each_svm_dev(sdev, svm, d) \
> + list_for_each_entry((sdev), &(svm)->devs, list) \
> + if ((d) != (sdev)->dev) {} else
> +
> int intel_svm_bind_mm(struct device *dev, int *pasid, int flags, struct
> svm_dev_ops *ops)
> {
> struct intel_iommu *iommu = intel_svm_device_to_iommu(dev);
> @@ -274,15 +278,13 @@ int intel_svm_bind_mm(struct device *dev, int *pasid,
> int flags, struct svm_dev_
> goto out;
> }
>
> - list_for_each_entry(sdev, &svm->devs, list) {
> - if (dev == sdev->dev) {
> - if (sdev->ops != ops) {
> - ret = -EBUSY;
> - goto out;
> - }
> - sdev->users++;
> - goto success;
> + for_each_svm_dev(sdev, svm, dev) {
> + if (sdev->ops != ops) {
> + ret = -EBUSY;
> + goto out;
> }
> + sdev->users++;
> + goto success;
> }
I think this does not read better as this is now a
for_each loop that exits the loop on the first match.
>
> break;
> @@ -427,43 +429,36 @@ int intel_svm_unbind_mm(struct device *dev, int pasid)
> goto out;
> }
>
> - if (!svm)
> - goto out;
> -
> - list_for_each_entry(sdev, &svm->devs, list) {
[]
> + for_each_svm_dev(sdev, svm, dev) {
I think this should not remove the !svm test above.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/4] dma-debug: Use pr_fmt()
On Mon, 2018-12-03 at 17:28 +, Robin Murphy wrote:
> Use pr_fmt() to generate the "DMA-API: " prefix consistently. This
> results in it being added to a couple of pr_*() messages which were
> missing it before, and for the err_printk() calls moves it to the actual
> start of the message instead of somewhere in the middle.
>
> Signed-off-by: Robin Murphy
> ---
>
> I chose not to refactor the existing split strings for minimal churn here.
>
> kernel/dma/debug.c | 74 --
> 1 file changed, 38 insertions(+), 36 deletions(-)
>
> diff --git a/kernel/dma/debug.c b/kernel/dma/debug.c
> index 231ca4628062..91b84140e4a5 100644
> --- a/kernel/dma/debug.c
> +++ b/kernel/dma/debug.c
> @@ -17,6 +17,8 @@
> * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> */
>
> +#define pr_fmt(fmt) "DMA-API: " fmt
> +
> #include
> #include
> #include
> @@ -234,7 +236,7 @@ static bool driver_filter(struct device *dev)
> error_count += 1; \
> if (driver_filter(dev) && \
> (show_all_errors || show_num_errors > 0)) { \
> - WARN(1, "%s %s: " format, \
> + WARN(1, pr_fmt("%s %s: ") format, \
>dev ? dev_driver_string(dev) : "NULL", \
>dev ? dev_name(dev) : "NULL", ## arg); \
> dump_entry_trace(entry);\
I think converting this WARN to
dev_err(dev, format, ##__VA_ARGS__);
dump_stack();
would look better and be more intelligible.
Perhaps add a #define for dev_fmt if really necessary.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/amd: fix missing tag from dev_err message
On Tue, 2018-07-03 at 18:57 +0200, Walter Harms wrote:
> It is only cosmetics but even the author got lost about the loose bracket.
> I would suggest to remove all the brackets or if needed to move the [ in the
> message. We have enought memory this days.
My suggestion would be to remove the separate dev_err
and use "Event log [" in each dev_err.
So here's an actual suggested patch that does a few things:
o Coalesce formats and realigns arguments
o Uses pr_fmt and dev_fmt to prefix "AMD-Vi: " to all messages
o Remove embedded "AMD-Vi: " from various formats
Some existing pr_ uses now are prefixed too.
---
drivers/iommu/amd_iommu.c | 69 +++
1 file changed, 34 insertions(+), 35 deletions(-)
diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 596b95c50051..01583a8ccaf9 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -17,6 +17,9 @@
* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
*/
+#define pr_fmt(fmt) "AMD-Vi: " fmt
+#define dev_fmt pr_fmt
+
#include
#include
#include
@@ -273,9 +276,8 @@ static u16 get_alias(struct device *dev)
return pci_alias;
}
- pr_info("AMD-Vi: Using IVRS reported alias %02x:%02x.%d "
- "for device %s[%04x:%04x], kernel reported alias "
- "%02x:%02x.%d\n", PCI_BUS_NUM(ivrs_alias), PCI_SLOT(ivrs_alias),
+ pr_info("Using IVRS reported alias %02x:%02x.%d for device
%s[%04x:%04x], kernel reported alias %02x:%02x.%d\n",
+ PCI_BUS_NUM(ivrs_alias), PCI_SLOT(ivrs_alias),
PCI_FUNC(ivrs_alias), dev_name(dev), pdev->vendor, pdev->device,
PCI_BUS_NUM(pci_alias), PCI_SLOT(pci_alias),
PCI_FUNC(pci_alias));
@@ -287,7 +289,7 @@ static u16 get_alias(struct device *dev)
if (pci_alias == devid &&
PCI_BUS_NUM(ivrs_alias) == pdev->bus->number) {
pci_add_dma_alias(pdev, ivrs_alias & 0xff);
- pr_info("AMD-Vi: Added PCI DMA alias %02x.%d for %s\n",
+ pr_info("Added PCI DMA alias %02x.%d for %s\n",
PCI_SLOT(ivrs_alias), PCI_FUNC(ivrs_alias),
dev_name(dev));
}
@@ -507,8 +509,8 @@ static void dump_dte_entry(u16 devid)
int i;
for (i = 0; i < 4; ++i)
- pr_err("AMD-Vi: DTE[%d]: %016llx\n", i,
- amd_iommu_dev_table[devid].data[i]);
+ pr_err("DTE[%d]: %016llx\n",
+ i, amd_iommu_dev_table[devid].data[i]);
}
static void dump_command(unsigned long phys_addr)
@@ -517,7 +519,7 @@ static void dump_command(unsigned long phys_addr)
int i;
for (i = 0; i < 4; ++i)
- pr_err("AMD-Vi: CMD[%d]: %08x\n", i, cmd->data[i]);
+ pr_err("CMD[%d]: %08x\n", i, cmd->data[i]);
}
static void amd_iommu_report_page_fault(u16 devid, u16 domain_id,
@@ -532,10 +534,10 @@ static void amd_iommu_report_page_fault(u16 devid, u16
domain_id,
dev_data = get_dev_data(&pdev->dev);
if (dev_data && __ratelimit(&dev_data->rs)) {
- dev_err(&pdev->dev, "AMD-Vi: Event logged [IO_PAGE_FAULT
domain=0x%04x address=0x%016llx flags=0x%04x]\n",
+ dev_err(&pdev->dev, "Event logged [IO_PAGE_FAULT domain=0x%04x
address=0x%016llx flags=0x%04x]\n",
domain_id, address, flags);
} else if (printk_ratelimit()) {
- pr_err("AMD-Vi: Event logged [IO_PAGE_FAULT device=%02x:%02x.%x
domain=0x%04x address=0x%016llx flags=0x%04x]\n",
+ pr_err("Event logged [IO_PAGE_FAULT device=%02x:%02x.%x
domain=0x%04x address=0x%016llx flags=0x%04x]\n",
PCI_BUS_NUM(devid), PCI_SLOT(devid), PCI_FUNC(devid),
domain_id, address, flags);
}
@@ -562,7 +564,7 @@ static void iommu_print_event(struct amd_iommu *iommu, void
*__evt)
if (type == 0) {
/* Did we hit the erratum? */
if (++count == LOOP_TIMEOUT) {
- pr_err("AMD-Vi: No event written to event log\n");
+ pr_err("No event written to event log\n");
return;
}
udelay(1);
@@ -572,43 +574,40 @@ static void iommu_print_event(struct amd_iommu *iommu,
void *__evt)
if (type == EVENT_TYPE_IO_FAULT) {
amd_iommu_report_page_fault(devid, pasid, address, flags);
return;
- } else {
- dev_err(dev, "AMD-Vi: Event logged [");
}
switch (type) {
case EVENT_TYPE_ILL_DEV:
- dev_err(dev, "ILLEGAL_DEV_TABLE_ENTRY device=%02x:%02x.%x
pasid=0x%05x address=0x%016llx flags=0x%04x]\n",
+ dev_err(dev, "Event logged [ILLEGAL_DEV_TABLE_ENTRY
device=%02x:%02x.%x pasid=0x%05x address=0x%016llx flags=0x%04x]\n",
PCI_BUS_
Re: [PATCH] iommu/amd: fix missing tag from dev_err message
On Tue, 2018-07-03 at 07:56 -0500, Gary R Hook wrote: > On 07/03/2018 05:07 AM, Joe Perches wrote: > > On Tue, 2018-07-03 at 07:40 +0100, Colin King wrote: > > > Currently tag is being assigned but not used, it is missing from > > > the dev_err message, so add it in. > > > > > > Cleans up clang warning: > > > warning: variable 'tag' set but not used [-Wunused-but-set-variable] > > > > [] > > > diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c > > > > [] > > > @@ -616,9 +616,9 @@ static void iommu_print_event(struct amd_iommu > > > *iommu, void *__evt) > > > pasid = ((event[0] >> 16) & 0x) > > > | ((event[1] << 6) & 0xF); > > > tag = event[1] & 0x03FF; > > > - dev_err(dev, "INVALID_PPR_REQUEST device=%02x:%02x.%x > > > pasid=0x%05x address=0x%016llx flags=0x%04x]\n", > > > + dev_err(dev, "INVALID_PPR_REQUEST device=%02x:%02x.%x > > > pasid=0x%05x address=0x%016llx flags=0x%04x tag=0x%03x]\n", > > > PCI_BUS_NUM(devid), PCI_SLOT(devid), > > > PCI_FUNC(devid), > > > - pasid, address, flags); > > > + pasid, address, flags, tag); > > > > Seems to have a superfluous ] that should be removed. > > Yeah, I pretty much messed up all of the log messages in that function. > My apologies. I'll create a patch for that problem; it shouldn't be > fixed here. I also wonder why event is declared volatile and then dereferenced with [] multiple times. Maybe each array dereference should be stored as a local variable instead. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/amd: fix missing tag from dev_err message
On Tue, 2018-07-03 at 11:27 -0500, Hook, Gary wrote:
> On 7/3/2018 11:24 AM, Colin Ian King wrote:
> > On 03/07/18 17:21, Hook, Gary wrote:
> > > On 7/3/2018 10:55 AM, Joe Perches wrote:
> > > > On Tue, 2018-07-03 at 07:56 -0500, Gary R Hook wrote:
> > > > > On 07/03/2018 05:07 AM, Joe Perches wrote:
> > > > > > On Tue, 2018-07-03 at 07:40 +0100, Colin King wrote:
> > > > > > > Currently tag is being assigned but not used, it is missing from
> > > > > > > the dev_err message, so add it in.
> > > > > > >
> > > > > > > Cleans up clang warning:
> > > > > > > warning: variable 'tag' set but not used
> > > > > > > [-Wunused-but-set-variable]
> > > > > >
> > > > > > []
> > > > > > > diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
> > > > > >
> > > > > > []
> > > > > > > @@ -616,9 +616,9 @@ static void iommu_print_event(struct amd_iommu
> > > > > > > *iommu, void *__evt)
> > > > > > > pasid = ((event[0] >> 16) & 0x)
> > > > > > > | ((event[1] << 6) & 0xF);
> > > > > > > tag = event[1] & 0x03FF;
> > > > > > > -dev_err(dev, "INVALID_PPR_REQUEST device=%02x:%02x.%x
> > > > > > > pasid=0x%05x address=0x%016llx flags=0x%04x]\n",
> > > > > > > +dev_err(dev, "INVALID_PPR_REQUEST device=%02x:%02x.%x
> > > > > > > pasid=0x%05x address=0x%016llx flags=0x%04x tag=0x%03x]\n",
> > > > > > > PCI_BUS_NUM(devid), PCI_SLOT(devid),
> > > > > > > PCI_FUNC(devid),
> > > > > > > -pasid, address, flags);
> > > > > > > +pasid, address, flags, tag);
> > > > > >
> > > > > > Seems to have a superfluous ] that should be removed.
> > > > >
> > > > > Yeah, I pretty much messed up all of the log messages in that
> > > > > function.
> > > > > My apologies. I'll create a patch for that problem; it shouldn't be
> > > > > fixed here.
> > >
> > > Well, no, I misremembered. The extraneous square brace has been there
> > > forever. Needs fixin', though.
> > >
> >
> > The opening square bracket is much earlier:
> >
> > dev_err(dev, "AMD-Vi: Event logged [");
> >
> > ..and all the subsequent dev_err messages have the trailing square bracket.
>
> Gah! Mystery solved. I'd forgotten about that.
>
> "Never mind."
>
It stems from the misconversion from printk to dev_err
The initial dev_err is fine but all the dev_err uses
in the switch/case should use pr_cont, not dev_err
(there's a suggested patch below this commit reference)
---
>From 90ca3859f5ea90050d00e695355934b37357e7bb Mon Sep 17 00:00:00 2001
From: Gary R Hook
Date: Thu, 8 Mar 2018 18:34:41 -0600
Subject: [PATCH] iommu/amd: Use dev_err to send events to the system log
Remove printk and use a more preferable error logging function.
Signed-off-by: Gary R Hook
Signed-off-by: Joerg Roedel
---
drivers/iommu/amd_iommu.c | 55
---
1 file changed, 28 insertions(+), 27 deletions(-)
diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 997a947ddc3b..4cd19f93ca15 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -547,6 +547,7 @@ static void amd_iommu_report_page_fault(u16 devid, u16
domain_id,
static void iommu_print_event(struct amd_iommu *iommu, void *__evt)
{
+ struct device *dev = iommu->iommu.dev;
int type, devid, domid, flags;
volatile u32 *event = __evt;
int count = 0;
@@ -573,53 +574,53 @@ static void iommu_print_event(struct amd_iommu *iommu,
void *__evt)
amd_iommu_report_page_fault(devid, domid, address, flags);
return;
} else {
- printk(KERN_ERR "AMD-Vi: Event logged [");
+ dev_err(dev, "AMD-Vi: Event logged [");
}
switch (type) {
case EVENT_TYPE_ILL_DEV:
- printk("ILLEGAL_DEV_TABLE_ENTRY device=%02x:%02x.%x "
- "address=0x%016llx flags=0x%04x]\n",
- PCI_BUS_NUM(devid), PCI_SLOT(devid), PCI_FUNC(devid),
- address, flags);
+ dev_err(dev,
Re: [PATCH] iommu/amd: fix missing tag from dev_err message
On Tue, 2018-07-03 at 07:40 +0100, Colin King wrote: > Currently tag is being assigned but not used, it is missing from > the dev_err message, so add it in. > > Cleans up clang warning: > warning: variable 'tag' set but not used [-Wunused-but-set-variable] [] > diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c [] > @@ -616,9 +616,9 @@ static void iommu_print_event(struct amd_iommu *iommu, > void *__evt) > pasid = ((event[0] >> 16) & 0x) > | ((event[1] << 6) & 0xF); > tag = event[1] & 0x03FF; > - dev_err(dev, "INVALID_PPR_REQUEST device=%02x:%02x.%x > pasid=0x%05x address=0x%016llx flags=0x%04x]\n", > + dev_err(dev, "INVALID_PPR_REQUEST device=%02x:%02x.%x > pasid=0x%05x address=0x%016llx flags=0x%04x tag=0x%03x]\n", > PCI_BUS_NUM(devid), PCI_SLOT(devid), PCI_FUNC(devid), > - pasid, address, flags); > + pasid, address, flags, tag); Seems to have a superfluous ] that should be removed. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 00/18] Convert default pr_fmt from empty to KBUILD_MODNAME
pr_ logging uses allow a prefix to be specified with a
specific #define pr_fmt
The default of pr_fmt in printk.h is #define pr_fmt(fmt) fmt
so no prefixing of logging output is generically done.
There are several output logging uses like dump_stack() that are
unprefixed and should remain unprefixed.
This patch series attempts to convert the default #define of pr_fmt to
KBUILD_MODNAME ": " fmt and as well update the various bits of the kernel
that should _not_ be prefixed by adding #define pr_fmt(fmt) fmt to those
compilation units that do not want output message prefixing.
There are about 1200 uses of #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
that could be removed if the default is changed.
A script that does this removal (and removes any blank lines that follow)
for the linux-kernel tree is included below:
$ git grep -P --name-only "define\s+pr_fmt\b" | \
grep -v include/linux/printk.h | \
xargs perl -i -e 'local $/; while (<>) {s/(\n)*[ \t]*#[ \t]*define[
\t]+pr_fmt[ \t]*\([ \t]*(\w+)[ \t]*\)[ \t]*KBUILD_MODNAME[ \t]*\": \"[ \t]*\2[
\t]*\n\s*/\1\n/; s/^\n//; print;}'
This script should be run after this patch series is applied.
The above script output diff is currently:
1198 files changed, 70 insertions(+), 2241 deletions(-)
Joe Perches (18):
kernel: Use pr_fmt
lib: Use pr_fmt
printk: Convert pr_fmt from blank define to KBUILD_MODNAME
x86: Remove pr_fmt duplicate logging prefixes
x86/mtrr: Rename main.c to mtrr.c and remove duplicate prefixes
net: Remove pr_fmt duplicate logging prefixes
blk-mq: Remove pr_fmt duplicate logging prefixes
random: Remove pr_fmt duplicate logging prefixes
ptp: Remove pr_fmt duplicate logging prefixes
efifb: Remove pr_fmt duplicate logging prefixes
proc: Remove pr_fmt duplicate logging prefixes
uprobes: Remove pr_fmt duplicate logging prefixes
printk: Remove pr_fmt duplicate logging prefixes
lib/mpi: Remove pr_fmt duplicate logging prefixes
security: Remove pr_fmt duplicate logging prefixes
aoe: Remove pr_fmt duplicate logging prefixes
security: encrypted-keys: Remove pr_fmt duplicate logging prefixes
rcu: Use pr_fmt to prefix "rcu: " to logging output
arch/x86/events/amd/ibs.c | 2 +-
arch/x86/kernel/cpu/mtrr/Makefile | 2 +-
arch/x86/kernel/cpu/mtrr/{main.c => mtrr.c}| 33 ++---
arch/x86/kernel/e820.c | 32 ++--
arch/x86/kernel/hpet.c | 5 +-
arch/x86/kernel/uprobes.c | 4 +-
arch/x86/mm/numa.c | 22 -
block/blk-mq.c | 9 ++--
drivers/block/aoe/aoeblk.c | 29 ++-
drivers/block/aoe/aoechr.c | 11 ++---
drivers/block/aoe/aoecmd.c | 34 ++---
drivers/block/aoe/aoedev.c | 19 +++-
drivers/block/aoe/aoemain.c| 6 +--
drivers/block/aoe/aoenet.c | 19 +++-
drivers/char/hw_random/via-rng.c | 10 ++--
drivers/char/random.c | 16 +++---
drivers/ptp/ptp_clock.c| 4 +-
drivers/video/fbdev/efifb.c| 48 +-
fs/proc/root.c | 6 +--
include/linux/printk.h | 2 +-
kernel/acct.c | 2 +
kernel/async.c | 14 +++---
kernel/audit_tree.c| 2 +-
kernel/backtracetest.c | 8 +--
kernel/crash_core.c| 29 ++-
kernel/events/uprobes.c| 3 +-
kernel/exit.c | 2 +
kernel/hung_task.c | 13 +++--
kernel/kprobes.c | 20 +---
kernel/module.c| 59 +++
kernel/panic.c | 3 ++
kernel/params.c| 13 +++--
kernel/pid.c | 2 +
kernel/printk/printk.c | 2 +-
kernel/profile.c | 2 +
kernel/range.c | 2 +-
kernel/rcu/rcu_segcblist.c | 2 +
kernel/rcu/rcuperf.c | 10 ++--
kernel/rcu/rcutorture.c| 46 +-
kernel/rcu/srcutiny.c | 2 +
kernel/rcu/srcutree.c | 5 +-
kernel/rcu/tiny.c | 3 ++
kernel/rcu/tree.c | 8 +--
kernel/rcu/tree_plugin.h | 67 +++---
kernel/rcu/update.c| 19 +---
kernel/relay.c |
[PATCH 02/18] lib: Use pr_fmt
Sometime in the future, it would be useful to convert pr_fmt from a
default simple define to use a default prefix with KBUILD_MODNAME.
There are files in lib/ that use pr_, some with an embedded
prefix, that also do not have a specific pr_fmt define.
Add pr_fmt for those files.
There are some differences in output as some logging output now is
prefixed with the appropriate KBUILD_MODNAME.
Miscellanea:
o Simplify debugging macros to only require a single function definition
by moving #define DEBUG into the definition and removing the empty
function definition
o Function alignment neatening
o Use %s, __func__ instead of embedding function names as strings
o Add missing newline terminations
Signed-off-by: Joe Perches
---
lib/cpu_rmap.c | 15 +---
lib/crc32test.c | 2 ++
lib/earlycpio.c | 5 ++--
lib/find_bit_benchmark.c | 2 ++
lib/kobject.c| 36 ++--
lib/kobject_uevent.c | 27 ++---
lib/nmi_backtrace.c | 3 +++
lib/percpu_ida.c | 4 +++-
lib/percpu_test.c| 2 ++
lib/random32.c | 10
lib/stmp_device.c| 2 ++
lib/string.c | 2 ++
lib/swiotlb.c| 4 +++-
lib/test_debug_virtual.c | 2 ++
lib/test_rhashtable.c| 44 ++
lib/test_sort.c | 2 ++
lib/ubsan.c | 61
17 files changed, 122 insertions(+), 101 deletions(-)
diff --git a/lib/cpu_rmap.c b/lib/cpu_rmap.c
index f610b2a10b3e..2d7204928c60 100644
--- a/lib/cpu_rmap.c
+++ b/lib/cpu_rmap.c
@@ -7,6 +7,8 @@
* by the Free Software Foundation, incorporated herein by reference.
*/
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
#include
#include
#include
@@ -110,26 +112,21 @@ static bool cpu_rmap_copy_neigh(struct cpu_rmap *rmap,
unsigned int cpu,
return false;
}
-#ifdef DEBUG
static void debug_print_rmap(const struct cpu_rmap *rmap, const char *prefix)
{
+#ifdef DEBUG
unsigned index;
unsigned int cpu;
- pr_info("cpu_rmap %p, %s:\n", rmap, prefix);
+ pr_info("%p, %s:\n", rmap, prefix);
for_each_possible_cpu(cpu) {
index = rmap->near[cpu].index;
pr_info("cpu %d -> obj %u (distance %u)\n",
cpu, index, rmap->near[cpu].dist);
}
-}
-#else
-static inline void
-debug_print_rmap(const struct cpu_rmap *rmap, const char *prefix)
-{
-}
#endif
+}
/**
* cpu_rmap_add - add object to a rmap
@@ -258,7 +255,7 @@ irq_cpu_rmap_notify(struct irq_affinity_notify *notify,
const cpumask_t *mask)
rc = cpu_rmap_update(glue->rmap, glue->index, mask);
if (rc)
- pr_warning("irq_cpu_rmap_notify: update failed: %d\n", rc);
+ pr_warn("%s: update failed: %d\n", __func__, rc);
}
/**
diff --git a/lib/crc32test.c b/lib/crc32test.c
index 97d6a57cefcc..63bb08ccb6f3 100644
--- a/lib/crc32test.c
+++ b/lib/crc32test.c
@@ -24,6 +24,8 @@
* Version 2. See the file COPYING for more details.
*/
+#define pr_fmt(fmt) fmt
+
#include
#include
#include
diff --git a/lib/earlycpio.c b/lib/earlycpio.c
index db283ba4d2c1..e98816b13719 100644
--- a/lib/earlycpio.c
+++ b/lib/earlycpio.c
@@ -130,9 +130,8 @@ struct cpio_data find_cpio_data(const char *path, void
*data,
*nextoff = (long)nptr - (long)data;
if (ch[C_NAMESIZE] - mypathsize >= MAX_CPIO_FILE_NAME) {
- pr_warn(
- "File %s exceeding MAX_CPIO_FILE_NAME [%d]\n",
- p, MAX_CPIO_FILE_NAME);
+ pr_warn("File %s exceeding MAX_CPIO_FILE_NAME
[%d]\n",
+ p, MAX_CPIO_FILE_NAME);
}
strlcpy(cd.name, p + mypathsize, MAX_CPIO_FILE_NAME);
diff --git a/lib/find_bit_benchmark.c b/lib/find_bit_benchmark.c
index 5367ffa5c18f..d59d92c9e73e 100644
--- a/lib/find_bit_benchmark.c
+++ b/lib/find_bit_benchmark.c
@@ -24,6 +24,8 @@
* - sparse bitmap with few set bits at random positions.
*/
+#define pr_fmt(fmt) fmt
+
#include
#include
#include
diff --git a/lib/kobject.c b/lib/kobject.c
index 18989b5b3b56..2bb9a50da4b0 100644
--- a/lib/kobject.c
+++ b/lib/kobject.c
@@ -10,6 +10,8 @@
* about using the kobject interface.
*/
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
#include
#include
#include
@@ -129,8 +131,8 @@ static void fill_kobj_path(struct kobject *kobj, char
*path, int length)
*(path + --length) = '/';
}
- pr_debug("kobject: '%s' (%p): %s: path = '%s'\n", kobject_name(kobj),
-kobj, __func__, path);
+ pr_debug("
Re: [PATCH v5 1/2] iommu - Enable debugfs exposure of IOMMU driver internals
On Tue, 2018-05-08 at 15:07 -0500, Gary R Hook wrote:
> On 05/08/2018 01:48 PM, Joe Perches wrote:
> > On Tue, 2018-05-08 at 12:08 -0500, Hook, Gary wrote:
> > > On 5/7/2018 6:47 PM, kbuild test robot wrote:
> > > >
> > > > All error/warnings (new ones prefixed by >>):
> > > >
> > > > In file included from include/linux/intel-iommu.h:32:0,
> > > > from drivers/gpu/drm/i915/i915_drv.h:41,
> > > > from drivers/gpu/drm/i915/i915_oa_bxt.c:31:
> > > > include/linux/iommu.h: In function 'iommu_debugfs_new_driver_dir':
> > > > > > include/linux/iommu.h:706:8: error: parameter name omitted
> > > >
> > > > struct dentry *iommu_debugfs_new_driver_dir(char *) {};
> > > > ^~
> > > > In file included from include/linux/intel-iommu.h:32:0,
> > > > from drivers/gpu/drm/i915/i915_drv.h:41,
> > > > from drivers/gpu/drm/i915/i915_oa_bxt.c:31:
> > > > > > include/linux/iommu.h:706:8: warning: control reaches end of
> > > > > > non-void function [-Wreturn-type]
> > > >
> > > > struct dentry *iommu_debugfs_new_driver_dir(char *) {};
> > > > ^~
> > > >
> > > > vim +706 include/linux/iommu.h
> > > >
> > > > 700
> > > > 701#ifdef CONFIG_IOMMU_DEBUGFS
> > > > 702void iommu_debugfs_setup(void);
> > > > 703struct dentry *iommu_debugfs_new_driver_dir(char *);
> > > > 704#else
> > > > 705static inline void iommu_debugfs_setup(void) {}
> > > >> 706struct dentry *iommu_debugfs_new_driver_dir(char *) {};
> > > > 707#endif
> > > > 708
> > >
> > > I have no problems with adding parameter names. But
> > > scripts/checkpatch.pl doesn't seem to check for this, nor require it.
> > > Should checkpatch be updated?
> >
> > I'm pretty sure that's not feasible.
>
> Ugh. This is a definition, not a declaration. My bad. Which is likely
> why I decided to apologize up front.
>
> > And when the compiler tells you you've stuffed up some
> > syntactical bit, why should checkpatch duplicate the
> > output error message too?
>
> Well, that's the point: neither the 4.8 nor 5.4 compiler complained
> about this.
Perhaps because CONFIG_IOMMU_DEBUGFS was set in the .config
for all the compilation previously performed?
> Not as an error, despite the fact that (now that I read what
> is actually here, as opposed to what I think is there) this is wrong.
> Had an error message been emitted, and the make stopped, I would have
> figure this out before embarrassing myself in front of the entire interwebs.
There's no reason for that figuring out to be necessary.
Linux compilation complexity is pretty high and almost
no one understands it completely.
cheers, Joe
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 1/2] iommu - Enable debugfs exposure of IOMMU driver internals
On Tue, 2018-05-08 at 12:08 -0500, Hook, Gary wrote:
> On 5/7/2018 6:47 PM, kbuild test robot wrote:
> > Hi Gary,
> >
>
> I imagine these questions have been asked before, so I'll ask for
> forgiveness up front.
>
>
> > Thank you for the patch! Yet something to improve:
> >
> > [auto build test ERROR on iommu/next]
> > [also build test ERROR on v4.17-rc4 next-20180507]
> > [if your patch is applied to the wrong git tree, please drop us a note to
> > help improve the system]
> >
> > url:
> > https://github.com/0day-ci/linux/commits/Gary-R-Hook/iommu-Enable-debugfs-exposure-of-IOMMU-driver-internals/20180508-062918
> > base: https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
> > config: x86_64-randconfig-x016-201818 (attached as .config)
> > compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
> > reproduce:
> > # save the attached .config to linux build tree
> > make ARCH=x86_64
>
> Is the gcc 7 compiler now a requirement to build the kernel? Or only to
> run krobot tests?
>
> Is this the earliest version of the compiler that can be used? I'm still
> using 4.8 and 5.4, which seems to suffice for the kernel.
>
> Googling about this has been fruitless.
>
> >
> > All error/warnings (new ones prefixed by >>):
> >
> > In file included from include/linux/intel-iommu.h:32:0,
> > from drivers/gpu/drm/i915/i915_drv.h:41,
> > from drivers/gpu/drm/i915/i915_oa_bxt.c:31:
> > include/linux/iommu.h: In function 'iommu_debugfs_new_driver_dir':
> > > > include/linux/iommu.h:706:8: error: parameter name omitted
> >
> > struct dentry *iommu_debugfs_new_driver_dir(char *) {};
> > ^~
> > In file included from include/linux/intel-iommu.h:32:0,
> > from drivers/gpu/drm/i915/i915_drv.h:41,
> > from drivers/gpu/drm/i915/i915_oa_bxt.c:31:
> > > > include/linux/iommu.h:706:8: warning: control reaches end of non-void
> > > > function [-Wreturn-type]
> >
> > struct dentry *iommu_debugfs_new_driver_dir(char *) {};
> > ^~
> >
> > vim +706 include/linux/iommu.h
> >
> > 700
> > 701 #ifdef CONFIG_IOMMU_DEBUGFS
> > 702 void iommu_debugfs_setup(void);
> > 703 struct dentry *iommu_debugfs_new_driver_dir(char *);
> > 704 #else
> > 705 static inline void iommu_debugfs_setup(void) {}
> > > 706 struct dentry *iommu_debugfs_new_driver_dir(char *) {};
> > 707 #endif
> > 708
>
> I have no problems with adding parameter names. But
> scripts/checkpatch.pl doesn't seem to check for this, nor require it.
> Should checkpatch be updated?
I'm pretty sure that's not feasible.
And when the compiler tells you you've stuffed up some
syntactical bit, why should checkpatch duplicate the
output error message too?
btw: That's an unnecessary ; at the end of that non-void
function and it should probably be something like:
static inline struct dentry *iommu_debugfs_new_driver_dir(char *dir)
{
return NULL;
}
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] fix double ;;s in code
On Tue, 2018-02-20 at 17:19 +1100, Michael Ellerman wrote: > Daniel Vetter writes: > > On Sun, Feb 18, 2018 at 11:00:56AM +0100, Christophe LEROY wrote: > > > Le 17/02/2018 à 22:19, Pavel Machek a écrit : > > > > > > > > Fix double ;;'s in code. > > > > > > > > Signed-off-by: Pavel Machek > > > > > > A summary of the files modified on top of the patch would help understand > > > the impact. > > > > > > A maybe there should be one patch by area, eg one for each arch specific > > > modif and one for drivers/ and one for block/ ? > > > > Yeah, pls split this into one patch per area, with a suitable patch > > subject prefix. Look at git log of each file to get a feeling for what's > > the standard in each area. > > This part is actually pretty annoying. > > I hacked up a script (below) which seems to do a reasonable job in most > cases. Here was another suggestion from awhile ago https://lkml.org/lkml/2010/11/16/245 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] Intel-IOMMU: Delete an error message for a failed memory allocation in init_dmars()
On Sun, 2018-01-21 at 08:19 +0100, Jörg Rödel wrote:
> On Sat, Jan 20, 2018 at 05:37:52PM -0800, Joe Perches wrote:
> > While Markus' commit messages are nearly universally terrible,
> > is there really any signficant value in knowing when any
> > particular OOM condition occurs other than the subsystem that
> > became OOM?
> >
> > You're going to be hosed in any case.
> >
> > Why isn't the generic OOM stack dump good enough?
>
> Because if we know the exact allocation attempt that failed right away,
> we can more easily check if we can rewrite it so that it is more likely
> to succeed, e.g. rewriting one higher-order allocation to multiple
> order-0 allocations.
Up to you but I think that's pretty dubious
as this is an init function and if it fails
the system really is stuffed.
Unrelated, there are some unnecessary casts
of pointers to void * that could be removed
to help make the code a bit more regular as
some callers use the cast and some do not.
---
drivers/iommu/intel-iommu.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 4a2de34895ec..8d7ea76345ae 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -845,8 +845,8 @@ static inline struct context_entry
*iommu_context_addr(struct intel_iommu *iommu
if (!context)
return NULL;
- __iommu_flush_cache(iommu, (void *)context, CONTEXT_SIZE);
- phy_addr = virt_to_phys((void *)context);
+ __iommu_flush_cache(iommu, context, CONTEXT_SIZE);
+ phy_addr = virt_to_phys(context);
*entry = phy_addr | 1;
__iommu_flush_cache(iommu, entry, sizeof(*entry));
}
@@ -1298,7 +1298,7 @@ static int iommu_alloc_root_entry(struct intel_iommu
*iommu)
struct root_entry *root;
unsigned long flags;
- root = (struct root_entry *)alloc_pgtable_page(iommu->node);
+ root = alloc_pgtable_page(iommu->node);
if (!root) {
pr_err("Allocating root entry for %s failed\n",
iommu->name);
@@ -1978,7 +1978,7 @@ static int domain_init(struct dmar_domain *domain, struct
intel_iommu *iommu,
domain->nid = iommu->node;
/* always allocate the top pgd */
- domain->pgd = (struct dma_pte *)alloc_pgtable_page(domain->nid);
+ domain->pgd = alloc_pgtable_page(domain->nid);
if (!domain->pgd)
return -ENOMEM;
__iommu_flush_cache(iommu, domain->pgd, PAGE_SIZE);
@@ -4168,7 +4168,7 @@ int __init dmar_parse_one_rmrr(struct acpi_dmar_header
*header, void *arg)
if (!rmrru->resv)
goto free_rmrru;
- rmrru->devices = dmar_alloc_dev_scope((void *)(rmrr + 1),
+ rmrru->devices = dmar_alloc_dev_scope(rmrr + 1,
((void *)rmrr) + rmrr->header.length,
&rmrru->devices_cnt);
if (rmrru->devices_cnt && rmrru->devices == NULL)
@@ -4229,7 +4229,7 @@ int dmar_parse_one_atsr(struct acpi_dmar_header *hdr,
void *arg)
memcpy(atsru->hdr, hdr, hdr->length);
atsru->include_all = atsr->flags & 0x1;
if (!atsru->include_all) {
- atsru->devices = dmar_alloc_dev_scope((void *)(atsr + 1),
+ atsru->devices = dmar_alloc_dev_scope(atsr + 1,
(void *)atsr + atsr->header.length,
&atsru->devices_cnt);
if (atsru->devices_cnt && atsru->devices == NULL) {
@@ -4465,7 +4465,7 @@ int dmar_iommu_notify_scope_dev(struct
dmar_pci_notify_info *info)
rmrr = container_of(rmrru->hdr,
struct acpi_dmar_reserved_memory, header);
if (info->event == BUS_NOTIFY_ADD_DEVICE) {
- ret = dmar_insert_dev_scope(info, (void *)(rmrr + 1),
+ ret = dmar_insert_dev_scope(info, rmrr + 1,
((void *)rmrr) + rmrr->header.length,
rmrr->segment, rmrru->devices,
rmrru->devices_cnt);
@@ -4483,7 +4483,7 @@ int dmar_iommu_notify_scope_dev(struct
dmar_pci_notify_info *info)
atsr = container_of(atsru->hdr, struct acpi_dmar_atsr, header);
if (info->event == BUS_NOTIFY_ADD_DEVICE) {
- ret = dmar_insert_dev_scope(info, (void *)(atsr + 1),
+ ret = dmar_insert_dev_scope(info, atsr + 1,
(void *)atsr + atsr->header.length,
atsr->segment, atsru->devices,
Re: [PATCH] Intel-IOMMU: Delete an error message for a failed memory allocation in init_dmars()
On Sat, 2018-01-20 at 20:40 +0100, Jörg Rödel wrote: > On Sat, Jan 20, 2018 at 03:55:37PM +0100, SF Markus Elfring wrote: > > Do you need any more background information for this general > > transformation pattern? > > No. > > > Do you find the Linux allocation failure report insufficient for this > > use case? > > Yes, because it can't tell me what the code was trying to allocate. While Markus' commit messages are nearly universally terrible, is there really any signficant value in knowing when any particular OOM condition occurs other than the subsystem that became OOM? You're going to be hosed in any case. Why isn't the generic OOM stack dump good enough? ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v10 1/9] perf/amd/iommu: Declare pr_fmt and remove unnecessary pr_debug
On Wed, 2017-02-15 at 14:56 -0600, Suravee Suthikulpanit wrote:
> Declare pr_fmt for perf/amd_iommu and remove unnecessary pr_debug.
[]
> diff --git 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
[]
> @@ -444,24 +440,24 @@ static __init int _init_perf_amd_iommu(
>
> raw_spin_lock_init(&perf_iommu->lock);
>
> - /* Init format attributes */
> perf_iommu->format_group = &amd_iommu_format_group;
>
> /* Init cpumask attributes to only core 0 */
> cpumask_set_cpu(0, &iommu_cpumask);
> perf_iommu->cpumask_group = &amd_iommu_cpumask_group;
>
> - /* Init events attributes */
> - if (_init_events_attrs(perf_iommu) != 0)
> - pr_err("perf: amd_iommu: Only support raw events.\n");
> + ret = _init_events_attrs(perf_iommu);
> + if (ret) {
> + pr_err("Error initializing AMD IOMMU perf events.\n");
> + return ret;
> + }
>
> - /* Init null attributes */
> perf_iommu->null_group = NULL;
> perf_iommu->pmu.attr_groups = perf_iommu->attr_groups;
>
> ret = perf_pmu_register(&perf_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",
You should remove the now unnecessary internal prefixes
in the other pr_ uses like the pr_info above.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/2] iommu/vt-d: Ratelimit fault handler
On Thu, 2016-03-17 at 14:12 -0600, Alex Williamson wrote: > Fault rates can easily overwhelm the console and make the system > unresponsive. Ratelimit to allow an opportunity for maintenance. [] > diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c [] > @@ -1602,10 +1602,17 @@ irqreturn_t dmar_fault(int irq, void *dev_id) > int reg, fault_index; > u32 fault_status; > unsigned long flag; > + bool ratelimited; > + static DEFINE_RATELIMIT_STATE(rs, > + DEFAULT_RATELIMIT_INTERVAL, > + DEFAULT_RATELIMIT_BURST); Are these the appropriate limits for dmar? include/linux/ratelimit.h:#define DEFAULT_RATELIMIT_INTERVAL(5 * HZ) include/linux/ratelimit.h:#define DEFAULT_RATELIMIT_BURST 10 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 0/2] iommu/vt-d: Fault logging improvements
On Thu, 2016-03-17 at 14:12 -0600, Alex Williamson wrote: > Ratelimit and improve formatting. Makes sense, thanks. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/vt-d: Ratelimit fault handler
On Tue, 2016-03-15 at 10:35 -0600, Alex Williamson wrote:
> Fault rates can easily overwhelm the console and make the system
> unresponsive. Ratelimit to allow an opportunity for maintenance.
A few suggestions:
o Use a single ratelimit state.
o The multiple lines output are unnecessary and hard to grep
in the dmesg output because of inconsistent prefixing as
second and subsequent output lines are not prefixed by pr_fmt.
o The DMAR prefix on the second block is also unnecessary as
it's already prefixed by pr_fmt
o Coalesce the formats for easier grep.
so maybe:
---
drivers/iommu/dmar.c | 28
1 file changed, 16 insertions(+), 12 deletions(-)
diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
index 8ffd756..59dcaaa 100644
--- a/drivers/iommu/dmar.c
+++ b/drivers/iommu/dmar.c
@@ -1575,23 +1575,27 @@ static int dmar_fault_do_one(struct intel_iommu *iommu,
int type,
{
const char *reason;
int fault_type;
+ static DEFINE_RATELIMIT_STATE(rs,
+ DEFAULT_RATELIMIT_INTERVAL,
+ DEFAULT_RATELIMIT_BURST);
+
+ if (__ratelimit(&rs))
+ return 0;
reason = dmar_get_fault_reason(fault_reason, &fault_type);
if (fault_type == INTR_REMAP)
- pr_err("INTR-REMAP: Request device [[%02x:%02x.%d] "
- "fault index %llx\n"
- "INTR-REMAP:[fault reason %02d] %s\n",
- (source_id >> 8), PCI_SLOT(source_id & 0xFF),
- PCI_FUNC(source_id & 0xFF), addr >> 48,
- fault_reason, reason);
+ pr_err("[INTR-REMAP] Request device [%02x:%02x.%d] fault index
%llx [fault reason %02d] %s\n",
+ source_id >> 8, PCI_SLOT(source_id & 0xFF),
+ PCI_FUNC(source_id & 0xFF), addr >> 48,
+ fault_reason, reason);
else
- pr_err("DMAR:[%s] Request device [%02x:%02x.%d] "
- "fault addr %llx \n"
- "DMAR:[fault reason %02d] %s\n",
- (type ? "DMA Read" : "DMA Write"),
- (source_id >> 8), PCI_SLOT(source_id & 0xFF),
- PCI_FUNC(source_id & 0xFF), addr, fault_reason, reason);
+ pr_err("[%s] Request device [%02x:%02x.%d] fault addr %llx
[fault reason %02d] %s\n",
+ type ? "DMA Read" : "DMA Write",
+ source_id >> 8, PCI_SLOT(source_id & 0xFF),
+ PCI_FUNC(source_id & 0xFF), addr,
+ fault_reason, reason);
+
return 0;
}
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/vt-d: Ratelimit fault handler
On Tue, 2016-03-15 at 10:10 -0700, Joe Perches wrote: > o Use a single ratelimit state. [] > diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c [] > + if (__ratelimit(&rs)) > + return 0; That of course should be: if (!__ratelimit(&rs)) return 0; ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu: Update my email address
On Wed, 2015-02-04 at 16:17 +0100, Joerg Roedel wrote: > The AMD address is dead for a long time already, replace it > with a working one. Maybe add a .mailmap entry too. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] omap: iommu: Fix decimal permissions
These should have been octal
Signed-off-by: Joe Perches
---
drivers/iommu/omap-iommu-debug.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/omap-iommu-debug.c b/drivers/iommu/omap-iommu-debug.c
index d97fbe4..80fffba 100644
--- a/drivers/iommu/omap-iommu-debug.c
+++ b/drivers/iommu/omap-iommu-debug.c
@@ -354,8 +354,8 @@ DEBUG_FOPS(mem);
return -ENOMEM; \
}
-#define DEBUG_ADD_FILE(name) __DEBUG_ADD_FILE(name, 600)
-#define DEBUG_ADD_FILE_RO(name) __DEBUG_ADD_FILE(name, 400)
+#define DEBUG_ADD_FILE(name) __DEBUG_ADD_FILE(name, 0600)
+#define DEBUG_ADD_FILE_RO(name) __DEBUG_ADD_FILE(name, 0400)
static int iommu_debug_register(struct device *dev, void *data)
{
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/1] x86/iommu: use bit structures for context_entry
On Fri, 2013-05-24 at 08:22 +0800, Li, Zhen-Hua wrote:
> There is a structure named context_entry used by intel iommu, and there
> are some bit operations on it. Use bit structure may make these operations
> easy.
[]
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
[]
> @@ -221,55 +221,28 @@ get_context_addr_from_root(struct root_entry *root)
> * 8-23: domain id
> */
> struct context_entry {
> - u64 lo;
> - u64 hi;
> + union {
> + struct {
> + u64 present:1,
why use struct and union at all?
Why not just:
struct context_entry {
u64 present:1,
fpd:1,
translation_type:2,
reserved_l:8,
asr:52;
u64 aw:3,
avail:4,
reserved_h1:1,
did:16,
reserved_h2:40;
};
> @@ -743,7 +716,8 @@ static void clear_context_table(struct intel_iommu
> *iommu, u8 bus, u8 devfn)
[]
> if (context) {
> - context_clear_entry(&context[devfn]);
> + context[devfn].lo = 0;
> + context[devfn].hi = 0;
memset(&context[devfn], 0, sizeof(...))
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 06/28] iommu/amd: Move informational prinks out of iommu_enable
On Thu, 2012-07-05 at 14:36 +0200, Joerg Roedel wrote:
> This function will be called before the PCI subsystem is
> initialized. Therefore dev_name doen't work and IOMMU
> information can't be printed to the klog as before. Move the
> code to print that information to a later point where PCI
> initializtion has already happened.
[]
> diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
[]
> @@ -1102,6 +1085,28 @@ static int iommu_init_pci(struct amd_iommu *iommu)
[]
> +static void print_iommu_info(void)
> +{
> + static const char * const feat_str[] = {
> + "PreF", "PPR", "X2APIC", "NX", "GT", "[5]",
> + "IA", "GA", "HE", "PC", NULL
> + };
> + struct amd_iommu *iommu;
> +
> + for_each_iommu(iommu) {
> + int i;
> + pr_info("AMD-Vi: Found IOMMU at %s cap 0x%hx\n",
> + dev_name(&iommu->dev->dev), iommu->cap_ptr);
> + if (iommu->cap & (1 << IOMMU_CAP_EFR)) {
> + pr_info("AMD-Vi: Extended features: ");
> + for (i = 0; feat_str[i]; ++i)
> + if (iommu_feature(iommu, (1ULL << i)))
> + pr_cont(" %s", feat_str[i]);
I think this should use {} around the for loop
and this would be better as:
static const char * const feat_str[] = {
"PreF", "PPR", "X2APIC", "NX", "GT", "[5]",
"IA", "GA", "HE", "PC"
};
[]
for (i = 0; ARRAY_SIZE(feat_str); i++) {
if (iommu_feature(iommu, (1ULL << i)))
pr_cont(" %s", feat_str[i]);
}
I don't see the utility of the separate function and
this could just be inlined in the calling function.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/2] iommu: dmar: replace printks with appropriate pr_*()
On Mon, 2012-06-04 at 17:29 -0400, Donald Dutile wrote:
> Replace printk(KERN_* with pr_*() functions.
Please add
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
before any include and remove the embedded PREFIX
from each printk
> diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
[]
> break;
> }
> pdev = pci_get_slot(bus, PCI_DEVFN(path->dev, path->fn));
> if (!pdev) {
> - printk(KERN_WARNING PREFIX
> - "Device scope device [%04x:%02x:%02x.%02x] not found\n",
> + pr_warn(PREFIX "Device scope device"
> + "[%04x:%02x:%02x.%02x] not found\n",
> segment, bus->number, path->dev, path->fn);
Please don't split any format string. You removed
a space between the scope device and an open bracket.
It's OK for format strings to exceed 80 chars.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
