Re: [PATCH 1/2] dma-mapping: zero memory returned from dma_alloc_*

[Greg Ungerer]

Hi Christoph,

On 17/12/18 9:59 pm, Christoph Hellwig wrote:

On Sat, Dec 15, 2018 at 12:14:29AM +1000, Greg Ungerer wrote:

Yep, that is right. Certainly the MMU case is broken. Some noMMU cases work
by virtue of the SoC only having an instruction cache (the older V2 cores).


Is there a good an easy case to detect if a core has a cache?  Either
runtime or in Kconfig?


The MMU case is fixable, but I think it will mean changing away from
the fall-back virtual:physical 1:1 mapping it uses for the kernel address
space. So not completely trivial. Either that or a dedicated area of RAM
for coherent allocations that we can mark as non-cachable via the really
course grained and limited ACR registers - not really very appealing.


What about CF_PAGE_NOCACHE?  Reading arch/m68k/include/asm/mcf_pgtable.h
suggest this would cause an uncached mapping, in which case something
like this should work:


http://git.infradead.org/users/hch/misc.git/commitdiff/4b8711d436e8d56edbc5ca19aa2be639705bbfef


No, that won't work.

The current MMU setup for ColdFire relies on a quirk of the cache
control subsystem to map kernel mapping (actually all of RAM when
accessed in supervisor mode).

The effective address calculation by the CPU/MMU firstly checks
the RAMBAR access, then

From the ColdFire 5475 Reference Manual (section 5.5.1):

  If virtual mode is enabled, any normal mode access that does not hit in the 
MMUBAR,
  RAMBARs, ROMBARs, or ACRs is considered a normal mode virtual address request 
and
  generates its access attributes from the MMU. For this case, the default CACR 
address attributes
  are not used.

The MMUBAR is the MMU control registers, the RAMBAR/ROMBAR are the
internal static RAM/ROM regions and the ACR are the cache control
registers. The code in arch/m68k/coldfire/head.S sets up the ACR
registers so that all of RAM is accessible and cached when in supervisor
mode. So kernel code and data accesses will hit this and use the
address for access. User pages won't hit this and will go through to
hit the MMU mappings.

The net out is we don't need page mappings or use TLB entries
for kernel code/data. The problem is we also can't map individual
regions as not cached for coherent allocations... The ACR mapping
means all-or-nothing.

This leads back to what I mentioned earlier about changing the
VM mapping to not use the ACR mapping method and actually page
mapping the kernel space. Not completely trivial and I expect
there will be a performance hit with the extra TLB pressure and
their setup/remapping overhead.



The noMMU case in general is probably limited to something like that same
type of dedicated RAM/ACR register mechamism.

The most commonly used periperal with DMA is the FEC ethernet module,
and it has some "special" (used very loosely) cache flushing for
parts like the 532x family which probably makes it mostly work right.
There is a PCI bus on the 54xx family of parts, and I know general
ethernet cards on it (like e1000's) have problems I am sure are
related to the fact that coherent memory allocations aren't.


If we really just care about FEC we can just switch it do use
DMA_ATTR_NON_CONSISTENT and do explicit cache flushing.  But as far
as I can tell FEC only uses DMA coherent allocations for the TSO
headers anyway, is TSO even used on this SOC?


The FEC is the most commonly used, but not the only. I test generic PCI
NICs on the PCI bus on the ColdFire 5475 - and a lot of those drivers
rely on coherent allocations.

Regards
Greg


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

[PATCH 1/1] iommu/vt-d: Support page request in scalable mode

[Lu Baolu]

From: Jacob Pan 

VT-d Rev3.0 has made a few changes to the page request interface,

1. widened PRQ descriptor from 128 bits to 256 bits;
2. removed streaming response type;
3. introduced private data that requires page response even the
   request is not last request in group (LPIG).

This is a supplement to commit 1c4f88b7f1f92 ("iommu/vt-d: Shared
virtual address in scalable mode") and makes the svm code compliant
with VT-d Rev3.0.

Cc: Ashok Raj 
Cc: Liu Yi L 
Cc: Kevin Tian 
Signed-off-by: Jacob Pan 
Fixes: 1c4f88b7f1f92 ("iommu/vt-d: Shared virtual address in scalable mode")
Signed-off-by: Lu Baolu 
---
 drivers/iommu/intel-svm.c   | 77 ++---
 include/linux/intel-iommu.h | 21 +-
 include/linux/intel-svm.h   |  2 +-
 3 files changed, 55 insertions(+), 45 deletions(-)

diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
index a2a2aa4439aa..79add5716552 100644
--- a/drivers/iommu/intel-svm.c
+++ b/drivers/iommu/intel-svm.c
@@ -470,20 +470,31 @@ EXPORT_SYMBOL_GPL(intel_svm_is_pasid_valid);
 
 /* Page request queue descriptor */
 struct page_req_dsc {
-   u64 srr:1;
-   u64 bof:1;
-   u64 pasid_present:1;
-   u64 lpig:1;
-   u64 pasid:20;
-   u64 bus:8;
-   u64 private:23;
-   u64 prg_index:9;
-   u64 rd_req:1;
-   u64 wr_req:1;
-   u64 exe_req:1;
-   u64 priv_req:1;
-   u64 devfn:8;
-   u64 addr:52;
+   union {
+   struct {
+   u64 type:8;
+   u64 pasid_present:1;
+   u64 priv_data_present:1;
+   u64 rsvd:6;
+   u64 rid:16;
+   u64 pasid:20;
+   u64 exe_req:1;
+   u64 pm_req:1;
+   u64 rsvd2:10;
+   };
+   u64 qw_0;
+   };
+   union {
+   struct {
+   u64 rd_req:1;
+   u64 wr_req:1;
+   u64 lpig:1;
+   u64 prg_index:9;
+   u64 addr:52;
+   };
+   u64 qw_1;
+   };
+   u64 priv_data[2];
 };
 
 #define PRQ_RING_MASK ((0x1000 << PRQ_ORDER) - 0x10)
@@ -596,7 +607,7 @@ static irqreturn_t prq_event_thread(int irq, void *d)
/* Accounting for major/minor faults? */
rcu_read_lock();
list_for_each_entry_rcu(sdev, >devs, list) {
-   if (sdev->sid == PCI_DEVID(req->bus, req->devfn))
+   if (sdev->sid == req->rid)
break;
}
/* Other devices can go away, but the drivers are not permitted
@@ -609,33 +620,35 @@ static irqreturn_t prq_event_thread(int irq, void *d)
 
if (sdev && sdev->ops && sdev->ops->fault_cb) {
int rwxp = (req->rd_req << 3) | (req->wr_req << 2) |
-   (req->exe_req << 1) | (req->priv_req);
-   sdev->ops->fault_cb(sdev->dev, req->pasid, req->addr, 
req->private, rwxp, result);
+   (req->exe_req << 1) | (req->pm_req);
+   sdev->ops->fault_cb(sdev->dev, req->pasid, req->addr,
+   req->priv_data, rwxp, result);
}
/* We get here in the error case where the PASID lookup failed,
   and these can be NULL. Do not use them below this point! */
sdev = NULL;
svm = NULL;
no_pasid:
-   if (req->lpig) {
-   /* Page Group Response */
+   if (req->lpig || req->priv_data_present) {
+   /*
+* Per VT-d spec. v3.0 ch7.7, system software must
+* respond with page group response if private data
+* is present (PDP) or last page in group (LPIG) bit
+* is set. This is an additional VT-d feature beyond
+* PCI ATS spec.
+*/
resp.qw0 = QI_PGRP_PASID(req->pasid) |
-   QI_PGRP_DID((req->bus << 8) | req->devfn) |
+   QI_PGRP_DID(req->rid) |
QI_PGRP_PASID_P(req->pasid_present) |
+   QI_PGRP_PDP(req->pasid_present) |
+   QI_PGRP_RESP_CODE(result) |
QI_PGRP_RESP_TYPE;
resp.qw1 = QI_PGRP_IDX(req->prg_index) |
-   QI_PGRP_PRIV(req->private) |
-   QI_PGRP_RESP_CODE(result);
-   } else if (req->srr) {
-   /* Page Stream Response */
-   resp.qw0 = QI_PSTRM_IDX(req->prg_index) |
-   

Re: [PATCH 0/3] Fix virtio-blk issue with SWIOTLB

[Jason Wang]

On 2019/1/10 下午9:44, Joerg Roedel wrote:

Hi,

there is a problem with virtio-blk driven devices when
virtio-ring uses SWIOTLB through the DMA-API. This happens
for example in AMD-SEV enabled guests, where the guest RAM
is mostly encrypted and all emulated DMA has to happen
to/from the SWIOTLB aperture.

The problem is a limitation of the SWIOTLB implementation,
which does not support allocations larger than 256kb. When
the virtio-blk driver tries to read/write a block larger
than that, the allocation of the dma-handle fails and an IO
error is reported.

This patch-set adds a check to the virtio-code whether it
might be using SWIOTLB bounce buffering and limits the
maximum segment size in the virtio-blk driver in this case,
so that it doesn't try to do larger reads/writes.



Just wonder if my understanding is correct IOMMU_PLATFORM must be set 
for all virtio devices under AMD-SEV guests?


Thanks




Please review.

Thanks,

Joerg

Joerg Roedel (3):
   swiotlb: Export maximum allocation size
   virtio: Introduce virtio_max_dma_size()
   virtio-blk: Consider virtio_max_dma_size() for maximum segment size

  drivers/block/virtio_blk.c   | 10 ++
  drivers/virtio/virtio_ring.c | 11 +++
  include/linux/swiotlb.h  | 12 
  include/linux/virtio.h   |  2 ++
  4 files changed, 31 insertions(+), 4 deletions(-)


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

Re: [PATCH v4 2/2] dts: arm64/sdm845: Add node for arm,mmu-500

[Bjorn Andersson]

On Tue 08 Jan 03:18 PST 2019, Vivek Gautam wrote:

> 
> On 1/8/2019 12:29 PM, Bjorn Andersson wrote:
> > On Thu 11 Oct 02:49 PDT 2018, Vivek Gautam wrote:
> > 
> > > Add device node for arm,mmu-500 available on sdm845.
> > > This MMU-500 with single TCU and multiple TBU architecture
> > > is shared among all the peripherals except gpu.
> > > 
> > Hi Vivek,
> > 
> > Applying this patch together with UFS ([1] and [2]) ontop of v5.0-rc1
> > causes my MTP reboot once the UFSHCD module is inserted and probed.
> > Independently the patches seems to work fine.
> > 
> > Do you have any suggestion to why this would be?
> 
> 
> Hi Bjorn,
> 
> Enabling SMMU on sdm845 when you have UFS also enabled, would need addition
> of
> 'iommus' property to ufs dt node.
> You will need to add the following with ufs:
> 
> iommus = <_smmu 0x100 0xf>;
> 

Thanks, this do address the sudden restart of my MTP, but provides a
fault.

[7.391117] arm-smmu 1500.iommu: Unhandled context fault: fsr=0x402, 
iova=0xdf3e0, fsynr=0x29, cb=0
[7.747406] ufshcd-qcom 1d84000.ufshc: ufshcd_verify_dev_init: NOP OUT 
failed -11

The only thing done ontop of v5.0-rc1, is to take your patch adding
apps_smmu, add the ufs nodes as Evan proposed and specify iommus in the
ufshcd node.


With Coreboot UFS seems to work without specifying iommus, but with it
UFS fails to come up.

Regards,
Bjorn

> Thanks
> Vivek
> 
> > 
> > [1] 
> > https://lore.kernel.org/lkml/20181210192826.241350-4-evgr...@chromium.org/
> > [2] 
> > https://lore.kernel.org/lkml/20181210192826.241350-5-evgr...@chromium.org/
> > 
> > Regards,
> > Bjorn
> > 
> > > Signed-off-by: Vivek Gautam 
> > > ---
> > > 
> > > Changes since v3:
> > >   - none.
> > > 
> > >   arch/arm64/boot/dts/qcom/sdm845.dtsi | 72 
> > > 
> > >   1 file changed, 72 insertions(+)
> > > 
> > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi 
> > > b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > > index b72bdb0a31a5..0aace729643d 100644
> > > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > > @@ -1297,6 +1297,78 @@
> > >   cell-index = <0>;
> > >   };
> > > + apps_smmu: iommu@1500 {
> > > + compatible = "qcom,sdm845-smmu-500", "arm,mmu-500";
> > > + reg = <0x1500 0x8>;
> > > + #iommu-cells = <2>;
> > > + #global-interrupts = <1>;
> > > + interrupts = ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +  ,
> > > +

Re: use generic DMA mapping code in powerpc V4

[Christian Zigotzky]

Next step: 891dcc1072f1fa27a83da920d88daff6ca08fc02 (powerpc/dma: remove 
dma_nommu_dma_supported)


git clone git://git.infradead.org/users/hch/misc.git -b powerpc-dma.6 a

git checkout 891dcc1072f1fa27a83da920d88daff6ca08fc02

Output:

Note: checking out '891dcc1072f1fa27a83da920d88daff6ca08fc02'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by performing another checkout.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -b with the checkout command again. Example:

git checkout -b 

HEAD is now at 891dcc1... powerpc/dma: remove dma_nommu_dma_supported

---

Link to the Git: 
http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/powerpc-dma.6


Results: PASEMI onboard ethernet works and the X5000 (P5020 board) 
boots. I also successfully tested sound, hardware 3D acceleration, 
Bluetooth, network, booting with a label etc. The uImages work also in a 
virtual e5500 quad-core QEMU machine.


-- Christian


On 09 January 2019 at 10:31AM, Christian Zigotzky wrote:
Next step: a64e18ba191ba9102fb174f27d707485ffd9389c (powerpc/dma: 
remove dma_nommu_get_required_mask)


git clone git://git.infradead.org/users/hch/misc.git -b powerpc-dma.6 a

git checkout a64e18ba191ba9102fb174f27d707485ffd9389c

Link to the Git: 
http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/powerpc-dma.6


Results: PASEMI onboard ethernet works and the X5000 (P5020 board) 
boots. I also successfully tested sound, hardware 3D acceleration, 
Bluetooth, network, booting with a label etc. The uImages work also in 
a virtual e5500 quad-core QEMU machine.


-- Christian


On 05 January 2019 at 5:03PM, Christian Zigotzky wrote:
Next step: c446404b041130fbd9d1772d184f24715cf2362f (powerpc/dma: 
remove dma_nommu_mmap_coherent)


git clone git://git.infradead.org/users/hch/misc.git -b powerpc-dma.6 a

git checkout c446404b041130fbd9d1772d184f24715cf2362f

Output:

Note: checking out 'c446404b041130fbd9d1772d184f24715cf2362f'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in 
this

state without impacting any branches by performing another checkout.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -b with the checkout command again. 
Example:


  git checkout -b 

HEAD is now at c446404... powerpc/dma: remove dma_nommu_mmap_coherent

-

Link to the Git: 
http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/powerpc-dma.6


Result: PASEMI onboard ethernet works and the X5000 (P5020 board) boots.

-- Christian






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

Re: [PATCH v2] kernel/dma: Fix panic caused by passing swiotlb to command line

[He Zhe]

On 1/11/19 9:46 AM, He Zhe wrote:
>
> On 1/11/19 1:27 AM, Konrad Rzeszutek Wilk wrote:
>> Let's skip it. There was another patch that would allocate a default 4MB 
>> size if it there was an misue of swiotlb parameters.
> But this patch mainly fixes a crash. Could you please point me to the patch 
> you mentioned?

And the v2 is modified according to your suggestion.

Zhe

>
> Thanks,
> Zhe
>
>>
>>
>> On Mon, Jan 7, 2019, 4:07 AM Christoph Hellwig >  wrote:
>>
>> On Mon, Jan 07, 2019 at 04:46:51PM +0800, He Zhe wrote:
>> > Kindly ping.
>>
>> Konrad, I'll pick this up through the DMA mapping tree unless you
>> protest in the next few days.
>>
>

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

Re: [PATCH v2] kernel/dma: Fix panic caused by passing swiotlb to command line

[He Zhe]

On 1/11/19 1:27 AM, Konrad Rzeszutek Wilk wrote:
> Let's skip it. There was another patch that would allocate a default 4MB size 
> if it there was an misue of swiotlb parameters.

But this patch mainly fixes a crash. Could you please point me to the patch you 
mentioned?

Thanks,
Zhe

>
>
>
> On Mon, Jan 7, 2019, 4:07 AM Christoph Hellwig   wrote:
>
> On Mon, Jan 07, 2019 at 04:46:51PM +0800, He Zhe wrote:
> > Kindly ping.
>
> Konrad, I'll pick this up through the DMA mapping tree unless you
> protest in the next few days.
>

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

Re: [RFC v3 14/21] iommu: introduce device fault data

[Jacob Pan]

On Tue,  8 Jan 2019 11:26:26 +0100
Eric Auger  wrote:

> From: Jacob Pan 
> 
> Device faults detected by IOMMU can be reported outside IOMMU
> subsystem for further processing. This patch intends to provide
> a generic device fault data such that device drivers can be
> communicated with IOMMU faults without model specific knowledge.
> 
> The proposed format is the result of discussion at:
> https://lkml.org/lkml/2017/11/10/291
> Part of the code is based on Jean-Philippe Brucker's patchset
> (https://patchwork.kernel.org/patch/9989315/).
> 
> The assumption is that model specific IOMMU driver can filter and
> handle most of the internal faults if the cause is within IOMMU driver
> control. Therefore, the fault reasons can be reported are grouped
> and generalized based common specifications such as PCI ATS.
> 
> Signed-off-by: Jacob Pan 
> Signed-off-by: Jean-Philippe Brucker 
> Signed-off-by: Liu, Yi L 
> Signed-off-by: Ashok Raj 
> Signed-off-by: Eric Auger 
> [moved part of the iommu_fault_event struct in the uapi, enriched
>  the fault reasons to be able to map unrecoverable SMMUv3 errors]
> ---
>  include/linux/iommu.h  | 55 -
>  include/uapi/linux/iommu.h | 83
> ++ 2 files changed, 136
> insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 244c1a3d5989..1dedc2d247c2 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -49,13 +49,17 @@ struct bus_type;
>  struct device;
>  struct iommu_domain;
>  struct notifier_block;
> +struct iommu_fault_event;
>  
>  /* iommu fault flags */
> -#define IOMMU_FAULT_READ 0x0
> -#define IOMMU_FAULT_WRITE0x1
> +#define IOMMU_FAULT_READ (1 << 0)
> +#define IOMMU_FAULT_WRITE(1 << 1)
> +#define IOMMU_FAULT_EXEC (1 << 2)
> +#define IOMMU_FAULT_PRIV (1 << 3)
>  
>  typedef int (*iommu_fault_handler_t)(struct iommu_domain *,
>   struct device *, unsigned long, int, void *);
> +typedef int (*iommu_dev_fault_handler_t)(struct iommu_fault_event *,
> void *); 
>  struct iommu_domain_geometry {
>   dma_addr_t aperture_start; /* First address that can be
> mapped*/ @@ -255,6 +259,52 @@ struct iommu_device {
>   struct device *dev;
>  };
>  
> +/**
> + * struct iommu_fault_event - Generic per device fault data
> + *
> + * - PCI and non-PCI devices
> + * - Recoverable faults (e.g. page request), information based on
> PCI ATS
> + * and PASID spec.
> + * - Un-recoverable faults of device interest
> + * - DMA remapping and IRQ remapping faults
> + *
> + * @fault: fault descriptor
> + * @device_private: if present, uniquely identify device-specific
> + *  private data for an individual page request.
> + * @iommu_private: used by the IOMMU driver for storing
> fault-specific
> + * data. Users should not modify this field before
> + * sending the fault response.
> + */
> +struct iommu_fault_event {
> + struct iommu_fault fault;
> + u64 device_private;
I think we want to move device_private to uapi since it gets injected
into the guest, then returned by guest in case of page response. For
VT-d we also need 128 bits of private data. VT-d spec. 7.7.1

For exception tracking (e.g. unanswered page request), I can add timer
and list info later when I include PRQ. sounds ok?
> + u64 iommu_private;

> +};
> +
> +/**
> + * struct iommu_fault_param - per-device IOMMU fault data
> + * @dev_fault_handler: Callback function to handle IOMMU faults at
> device level
> + * @data: handler private data
> + *
> + */
> +struct iommu_fault_param {
> + iommu_dev_fault_handler_t handler;
> + void *data;
> +};
> +
> +/**
> + * struct iommu_param - collection of per-device IOMMU data
> + *
> + * @fault_param: IOMMU detected device fault reporting data
> + *
> + * TODO: migrate other per device data pointers under
> iommu_dev_data, e.g.
> + *   struct iommu_group  *iommu_group;
> + *   struct iommu_fwspec *iommu_fwspec;
> + */
> +struct iommu_param {
> + struct iommu_fault_param *fault_param;
> +};
> +
>  int  iommu_device_register(struct iommu_device *iommu);
>  void iommu_device_unregister(struct iommu_device *iommu);
>  int  iommu_device_sysfs_add(struct iommu_device *iommu,
> @@ -438,6 +488,7 @@ struct iommu_ops {};
>  struct iommu_group {};
>  struct iommu_fwspec {};
>  struct iommu_device {};
> +struct iommu_fault_param {};
>  
>  static inline bool iommu_present(struct bus_type *bus)
>  {
> diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
> index f28cd9a1aa96..e9b5330a13c8 100644
> --- a/include/uapi/linux/iommu.h
> +++ b/include/uapi/linux/iommu.h
> @@ -148,4 +148,87 @@ struct iommu_guest_msi_binding {
>   __u64   gpa;
>   __u32   granule;
>  };
> +
> +/*  Generic fault types, can be expanded IRQ remapping fault */
> +enum iommu_fault_type {
> + 

Re: amdgpu/TTM oopses since merging swiotlb_dma_ops into the dma_direct code

[Sibren Vasse]

On Thu, 10 Jan 2019 at 15:48, Christoph Hellwig  wrote:
>
> On Thu, Jan 10, 2019 at 03:00:31PM +0100, Christian König wrote:
> >>  From the trace it looks like we git the case where swiotlb tries
> >> to copy back data from a bounce buffer, but hits a dangling or NULL
> >> pointer.  So a couple questions for the submitter:
> >>
> >>   - does the system have more than 4GB memory and thus use swiotlb?
> >> (check /proc/meminfo, and if something SWIOTLB appears in dmesg)
> >>   - does the device this happens on have a DMA mask smaller than
> >> the available memory, that is should swiotlb be used here to start
> >> with?
> >
> > Rather unlikely. The device is an AMD GPU, so we can address memory up to
> > 1TB.
>
> So we probably somehow got a false positive.
>
> For now I'like the reported to confirm that the dma_direct_unmap_page+0x92
> backtrace really is in the swiotlb code (I can't think of anything else,
> but I'd rather be sure).
I'm not sure what you want me to confirm. Could you elaborate?

>
> Second it would be great to print what the contents of io_tlb_start
> and io_tlb_end are, e.g. by doing a printk_once in is_swiotlb_buffer,
> maybe that gives a clue why we are hitting the swiotlb code here.

diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index 7c007ed7505f..042246dbae00 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -69,6 +69,7 @@ extern phys_addr_t io_tlb_start, io_tlb_end;

 static inline bool is_swiotlb_buffer(phys_addr_t paddr)
 {
+printk_once(KERN_INFO "io_tlb_start: %llu, io_tlb_end: %llu",
io_tlb_start, io_tlb_end);
 return paddr >= io_tlb_start && paddr < io_tlb_end;
 }

Result on boot:
[   11.405558] io_tlb_start: 3782983680, io_tlb_end: 3850092544

Regards,

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

Re: amdgpu/TTM oopses since merging swiotlb_dma_ops into the dma_direct code

[Sibren Vasse]

On Thu, 10 Jan 2019 at 18:06, Konrad Rzeszutek Wilk
 wrote:
>
> On Thu, Jan 10, 2019 at 04:26:43PM +0100, Sibren Vasse wrote:
> > On Thu, 10 Jan 2019 at 14:57, Christoph Hellwig  wrote:
> > >
> > > On Thu, Jan 10, 2019 at 10:59:02AM +0100, Michel Dänzer wrote:
> > > >
> > > > Hi Christoph,
> > > >
> > > >
> > > > https://bugs.freedesktop.org/109234 (please ignore comments #6-#9) was
> > > > bisected to your commit 55897af63091 "dma-direct: merge swiotlb_dma_ops
> > > > into the dma_direct code". Any ideas?
> > >
> > > From the trace it looks like we git the case where swiotlb tries
> > > to copy back data from a bounce buffer, but hits a dangling or NULL
> > > pointer.  So a couple questions for the submitter:
> > My apologies if I misunderstand something, this subject matter is new to me.
> >
> > >
> > >  - does the system have more than 4GB memory and thus use swiotlb?
> > My system has 8GB memory. The other report on the bug tracker had 16GB.
> >
> > >(check /proc/meminfo, and if something SWIOTLB appears in dmesg)
> > /proc/meminfo: https://ptpb.pw/4rxI
> > Can I grep dmesg for a string?
>
> Can you attach the 'dmesg'?
Dmesg attached.


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

Re: [PATCH v2] kernel/dma: Fix panic caused by passing swiotlb to command line

[Konrad Rzeszutek Wilk]

Let's skip it. There was another patch that would allocate a default 4MB
size if it there was an misue of swiotlb parameters.



On Mon, Jan 7, 2019, 4:07 AM Christoph Hellwig  On Mon, Jan 07, 2019 at 04:46:51PM +0800, He Zhe wrote:
> > Kindly ping.
>
> Konrad, I'll pick this up through the DMA mapping tree unless you
> protest in the next few days.
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: amdgpu/TTM oopses since merging swiotlb_dma_ops into the dma_direct code

[Konrad Rzeszutek Wilk]

On Thu, Jan 10, 2019 at 04:26:43PM +0100, Sibren Vasse wrote:
> On Thu, 10 Jan 2019 at 14:57, Christoph Hellwig  wrote:
> >
> > On Thu, Jan 10, 2019 at 10:59:02AM +0100, Michel Dänzer wrote:
> > >
> > > Hi Christoph,
> > >
> > >
> > > https://bugs.freedesktop.org/109234 (please ignore comments #6-#9) was
> > > bisected to your commit 55897af63091 "dma-direct: merge swiotlb_dma_ops
> > > into the dma_direct code". Any ideas?
> >
> > From the trace it looks like we git the case where swiotlb tries
> > to copy back data from a bounce buffer, but hits a dangling or NULL
> > pointer.  So a couple questions for the submitter:
> My apologies if I misunderstand something, this subject matter is new to me.
> 
> >
> >  - does the system have more than 4GB memory and thus use swiotlb?
> My system has 8GB memory. The other report on the bug tracker had 16GB.
> 
> >(check /proc/meminfo, and if something SWIOTLB appears in dmesg)
> /proc/meminfo: https://ptpb.pw/4rxI
> Can I grep dmesg for a string?

Can you attach the 'dmesg'? 

> 
> >  - does the device this happens on have a DMA mask smaller than
> >the available memory, that is should swiotlb be used here to start
> >with?
> It's a MSI Radeon RX 570 Gaming X 4GB. The other report was a RX 580.
> lshw output: https://ptpb.pw/6s0H
> 
> 
> Regards,
> 
> Sibren
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 1/3] swiotlb: Export maximum allocation size

[Konrad Rzeszutek Wilk]

On Thu, Jan 10, 2019 at 02:44:31PM +0100, Joerg Roedel wrote:
> From: Joerg Roedel 
> 
> The SWIOTLB implementation has a maximum size it can
> allocate dma-handles for. This needs to be exported so that
> device drivers don't try to allocate larger chunks.
> 
> This is especially important for block device drivers like
> virtio-blk, that might do DMA through SWIOTLB.

Why not use swiotlb_nr_tbl ? That is how drivers/gpu/drm use to figure if they
need to limit the size of pages.
> 
> Signed-off-by: Joerg Roedel 
> ---
>  include/linux/swiotlb.h | 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
> index 7c007ed7505f..0bcc80a97036 100644
> --- a/include/linux/swiotlb.h
> +++ b/include/linux/swiotlb.h
> @@ -72,6 +72,11 @@ static inline bool is_swiotlb_buffer(phys_addr_t paddr)
>   return paddr >= io_tlb_start && paddr < io_tlb_end;
>  }
>  
> +static inline size_t swiotlb_max_alloc_size(void)
> +{
> + return ((1UL << IO_TLB_SHIFT) * IO_TLB_SEGSIZE);
> +}
> +
>  bool swiotlb_map(struct device *dev, phys_addr_t *phys, dma_addr_t *dma_addr,
>   size_t size, enum dma_data_direction dir, unsigned long attrs);
>  void __init swiotlb_exit(void);
> @@ -95,6 +100,13 @@ static inline unsigned int swiotlb_max_segment(void)
>  {
>   return 0;
>  }
> +
> +static inline size_t swiotlb_max_alloc_size(void)
> +{
> + /* There is no limit when SWIOTLB isn't used */
> + return ~0UL;
> +}
> +
>  #endif /* CONFIG_SWIOTLB */
>  
>  extern void swiotlb_print_info(void);
> -- 
> 2.17.1
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: amdgpu/TTM oopses since merging swiotlb_dma_ops into the dma_direct code

[Sibren Vasse]

On Thu, 10 Jan 2019 at 14:57, Christoph Hellwig  wrote:
>
> On Thu, Jan 10, 2019 at 10:59:02AM +0100, Michel Dänzer wrote:
> >
> > Hi Christoph,
> >
> >
> > https://bugs.freedesktop.org/109234 (please ignore comments #6-#9) was
> > bisected to your commit 55897af63091 "dma-direct: merge swiotlb_dma_ops
> > into the dma_direct code". Any ideas?
>
> From the trace it looks like we git the case where swiotlb tries
> to copy back data from a bounce buffer, but hits a dangling or NULL
> pointer.  So a couple questions for the submitter:
My apologies if I misunderstand something, this subject matter is new to me.

>
>  - does the system have more than 4GB memory and thus use swiotlb?
My system has 8GB memory. The other report on the bug tracker had 16GB.

>(check /proc/meminfo, and if something SWIOTLB appears in dmesg)
/proc/meminfo: https://ptpb.pw/4rxI
Can I grep dmesg for a string?

>  - does the device this happens on have a DMA mask smaller than
>the available memory, that is should swiotlb be used here to start
>with?
It's a MSI Radeon RX 570 Gaming X 4GB. The other report was a RX 580.
lshw output: https://ptpb.pw/6s0H


Regards,

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

Re: amdgpu/TTM oopses since merging swiotlb_dma_ops into the dma_direct code

[Christoph Hellwig]

On Thu, Jan 10, 2019 at 03:00:31PM +0100, Christian König wrote:
>>  From the trace it looks like we git the case where swiotlb tries
>> to copy back data from a bounce buffer, but hits a dangling or NULL
>> pointer.  So a couple questions for the submitter:
>>
>>   - does the system have more than 4GB memory and thus use swiotlb?
>> (check /proc/meminfo, and if something SWIOTLB appears in dmesg)
>>   - does the device this happens on have a DMA mask smaller than
>> the available memory, that is should swiotlb be used here to start
>> with?
>
> Rather unlikely. The device is an AMD GPU, so we can address memory up to 
> 1TB.

So we probably somehow got a false positive.

For now I'like the reported to confirm that the dma_direct_unmap_page+0x92
backtrace really is in the swiotlb code (I can't think of anything else,
but I'd rather be sure).

Second it would be great to print what the contents of io_tlb_start
and io_tlb_end are, e.g. by doing a printk_once in is_swiotlb_buffer,
maybe that gives a clue why we are hitting the swiotlb code here.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 0/3] Fix virtio-blk issue with SWIOTLB

[Joerg Roedel]

Hi Christoph,

On Thu, Jan 10, 2019 at 02:59:52PM +0100, Christoph Hellwig wrote:
> On Thu, Jan 10, 2019 at 02:44:30PM +0100, Joerg Roedel wrote:

> > The problem is a limitation of the SWIOTLB implementation,
> > which does not support allocations larger than 256kb. When
> > the virtio-blk driver tries to read/write a block larger
> > than that, the allocation of the dma-handle fails and an IO
> > error is reported.
> 
> s/allocations/mappings/, right?  We don't use the swiotlb
> buffer for the coherent allocations anymore, and I don't think
> virtio-blk uses them anyway.

'Allocation' in the sense that there are address ranges allocated, not
memory, but mappings would also be right.

> I really don't like the fact that we hardcode swiotlb specific.
> This needs to be indirect through the dma ops or struct device,
> as various iommu implementations also have limits (although
> usually much larger ones).

I though about exposing it through DMA-API, but didn't go that route as
I didn't want to extend a generic API for some SWIOTLB specifics. But if
there are more implementations that have a size limitation it makes
certainly sense to put it into the DMA-API. I'll change that in the
next version.

Thanks,

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

Re: amdgpu/TTM oopses since merging swiotlb_dma_ops into the dma_direct code

[Christian König]

Am 10.01.19 um 14:57 schrieb Christoph Hellwig:

On Thu, Jan 10, 2019 at 10:59:02AM +0100, Michel Dänzer wrote:

Hi Christoph,


https://bugs.freedesktop.org/109234 (please ignore comments #6-#9) was
bisected to your commit 55897af63091 "dma-direct: merge swiotlb_dma_ops
into the dma_direct code". Any ideas?

 From the trace it looks like we git the case where swiotlb tries
to copy back data from a bounce buffer, but hits a dangling or NULL
pointer.  So a couple questions for the submitter:

  - does the system have more than 4GB memory and thus use swiotlb?
(check /proc/meminfo, and if something SWIOTLB appears in dmesg)
  - does the device this happens on have a DMA mask smaller than
the available memory, that is should swiotlb be used here to start
with?


Rather unlikely. The device is an AMD GPU, so we can address memory up 
to 1TB.


Christian.


___
dri-devel mailing list
dri-de...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


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

Re: [PATCH 0/3] Fix virtio-blk issue with SWIOTLB

[Christoph Hellwig]

On Thu, Jan 10, 2019 at 02:44:30PM +0100, Joerg Roedel wrote:
> Hi,
> 
> there is a problem with virtio-blk driven devices when
> virtio-ring uses SWIOTLB through the DMA-API. This happens
> for example in AMD-SEV enabled guests, where the guest RAM
> is mostly encrypted and all emulated DMA has to happen
> to/from the SWIOTLB aperture.
> 
> The problem is a limitation of the SWIOTLB implementation,
> which does not support allocations larger than 256kb. When
> the virtio-blk driver tries to read/write a block larger
> than that, the allocation of the dma-handle fails and an IO
> error is reported.

s/allocations/mappings/, right?  We don't use the swiotlb
buffer for the coherent allocations anymore, and I don't think
virtio-blk uses them anyway.

> This patch-set adds a check to the virtio-code whether it
> might be using SWIOTLB bounce buffering and limits the
> maximum segment size in the virtio-blk driver in this case,
> so that it doesn't try to do larger reads/writes.

I really don't like the fact that we hardcode swiotlb specific.
This needs to be indirect through the dma ops or struct device,
as various iommu implementations also have limits (although
usually much larger ones).
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: amdgpu/TTM oopses since merging swiotlb_dma_ops into the dma_direct code

[Christoph Hellwig]

On Thu, Jan 10, 2019 at 10:59:02AM +0100, Michel Dänzer wrote:
> 
> Hi Christoph,
> 
> 
> https://bugs.freedesktop.org/109234 (please ignore comments #6-#9) was
> bisected to your commit 55897af63091 "dma-direct: merge swiotlb_dma_ops
> into the dma_direct code". Any ideas?

>From the trace it looks like we git the case where swiotlb tries
to copy back data from a bounce buffer, but hits a dangling or NULL
pointer.  So a couple questions for the submitter:

 - does the system have more than 4GB memory and thus use swiotlb?
   (check /proc/meminfo, and if something SWIOTLB appears in dmesg)
 - does the device this happens on have a DMA mask smaller than
   the available memory, that is should swiotlb be used here to start
   with?
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH 3/3] virtio-blk: Consider virtio_max_dma_size() for maximum segment size

[Joerg Roedel]

From: Joerg Roedel 

Segments can't be larger than the maximum DMA allocation
size supported by virtio on the platform. Take that into
account when setting the maximum segment size for a block
device.

Signed-off-by: Joerg Roedel 
---
 drivers/block/virtio_blk.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index b16a887bbd02..6062faa8d213 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -723,7 +723,7 @@ static int virtblk_probe(struct virtio_device *vdev)
struct request_queue *q;
int err, index;
 
-   u32 v, blk_size, sg_elems, opt_io_size;
+   u32 v, blk_size, seg_size, sg_elems, opt_io_size;
u16 min_io_size;
u8 physical_block_exp, alignment_offset;
 
@@ -826,14 +826,16 @@ static int virtblk_probe(struct virtio_device *vdev)
/* No real sector limit. */
blk_queue_max_hw_sectors(q, -1U);
 
+   seg_size = virtio_max_dma_size(vdev);
+
/* Host can optionally specify maximum segment size and number of
 * segments. */
err = virtio_cread_feature(vdev, VIRTIO_BLK_F_SIZE_MAX,
   struct virtio_blk_config, size_max, );
if (!err)
-   blk_queue_max_segment_size(q, v);
-   else
-   blk_queue_max_segment_size(q, -1U);
+   seg_size = min(v, seg_size);
+
+   blk_queue_max_segment_size(q, seg_size);
 
/* Host can optionally specify the block size of the device */
err = virtio_cread_feature(vdev, VIRTIO_BLK_F_BLK_SIZE,
-- 
2.17.1

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

[PATCH 2/3] virtio: Introduce virtio_max_dma_size()

[Joerg Roedel]

From: Joerg Roedel 

This function returns the maximum segment size for a single
dma transaction of a virtio device. The possible limit comes
from the SWIOTLB implementation in the Linux kernel, that
has an upper limit of (currently) 256kb of contiguous
memory it can map.

The functions return the lower limie when SWIOTLB is used for DMA
transactions of the device and -1U otherwise.

Signed-off-by: Joerg Roedel 
---
 drivers/virtio/virtio_ring.c | 11 +++
 include/linux/virtio.h   |  2 ++
 2 files changed, 13 insertions(+)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index cd7e755484e3..c8d229da203e 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -266,6 +266,17 @@ static bool vring_use_dma_api(struct virtio_device *vdev)
return false;
 }
 
+size_t virtio_max_dma_size(struct virtio_device *vdev)
+{
+   const struct dma_map_ops *ops = get_dma_ops(>dev);
+   size_t max_segment_size = -1U;
+
+   if (vring_use_dma_api(vdev) && dma_is_direct(ops))
+   max_segment_size = swiotlb_max_alloc_size();
+
+   return max_segment_size;
+}
+
 static void *vring_alloc_queue(struct virtio_device *vdev, size_t size,
  dma_addr_t *dma_handle, gfp_t flag)
 {
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index fa1b5da2804e..673fe3ef3607 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -157,6 +157,8 @@ int virtio_device_freeze(struct virtio_device *dev);
 int virtio_device_restore(struct virtio_device *dev);
 #endif
 
+size_t virtio_max_dma_size(struct virtio_device *vdev);
+
 #define virtio_device_for_each_vq(vdev, vq) \
list_for_each_entry(vq, >vqs, list)
 
-- 
2.17.1

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

[PATCH 1/3] swiotlb: Export maximum allocation size

[Joerg Roedel]

From: Joerg Roedel 

The SWIOTLB implementation has a maximum size it can
allocate dma-handles for. This needs to be exported so that
device drivers don't try to allocate larger chunks.

This is especially important for block device drivers like
virtio-blk, that might do DMA through SWIOTLB.

Signed-off-by: Joerg Roedel 
---
 include/linux/swiotlb.h | 12 
 1 file changed, 12 insertions(+)

diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index 7c007ed7505f..0bcc80a97036 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -72,6 +72,11 @@ static inline bool is_swiotlb_buffer(phys_addr_t paddr)
return paddr >= io_tlb_start && paddr < io_tlb_end;
 }
 
+static inline size_t swiotlb_max_alloc_size(void)
+{
+   return ((1UL << IO_TLB_SHIFT) * IO_TLB_SEGSIZE);
+}
+
 bool swiotlb_map(struct device *dev, phys_addr_t *phys, dma_addr_t *dma_addr,
size_t size, enum dma_data_direction dir, unsigned long attrs);
 void __init swiotlb_exit(void);
@@ -95,6 +100,13 @@ static inline unsigned int swiotlb_max_segment(void)
 {
return 0;
 }
+
+static inline size_t swiotlb_max_alloc_size(void)
+{
+   /* There is no limit when SWIOTLB isn't used */
+   return ~0UL;
+}
+
 #endif /* CONFIG_SWIOTLB */
 
 extern void swiotlb_print_info(void);
-- 
2.17.1

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

[PATCH 0/3] Fix virtio-blk issue with SWIOTLB

[Joerg Roedel]

Hi,

there is a problem with virtio-blk driven devices when
virtio-ring uses SWIOTLB through the DMA-API. This happens
for example in AMD-SEV enabled guests, where the guest RAM
is mostly encrypted and all emulated DMA has to happen
to/from the SWIOTLB aperture.

The problem is a limitation of the SWIOTLB implementation,
which does not support allocations larger than 256kb. When
the virtio-blk driver tries to read/write a block larger
than that, the allocation of the dma-handle fails and an IO
error is reported.

This patch-set adds a check to the virtio-code whether it
might be using SWIOTLB bounce buffering and limits the
maximum segment size in the virtio-blk driver in this case,
so that it doesn't try to do larger reads/writes.

Please review.

Thanks,

Joerg

Joerg Roedel (3):
  swiotlb: Export maximum allocation size
  virtio: Introduce virtio_max_dma_size()
  virtio-blk: Consider virtio_max_dma_size() for maximum segment size

 drivers/block/virtio_blk.c   | 10 ++
 drivers/virtio/virtio_ring.c | 11 +++
 include/linux/swiotlb.h  | 12 
 include/linux/virtio.h   |  2 ++
 4 files changed, 31 insertions(+), 4 deletions(-)

-- 
2.17.1

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

Re: [PATCH 1/2] ACPI/IORT: Handle potential overflow in iort_dma_setup

[Auger Eric]

Hi Robin, Drew,

On 12/19/18 2:18 PM, Andrew Jones wrote:
> On Wed, Dec 19, 2018 at 12:21:35PM +, Robin Murphy wrote:
>> On 18/12/2018 18:48, Andrew Jones wrote:
>>> The sum of dmaaddr and size may overflow, particularly considering
>>> there are cases where size will be U64_MAX.
>>
>> Only if the firmware is broken in the first place, though. It would be weird
>> to describe an explicit _DMA range of base=0 and size=U64_MAX, because it's
>> effectively the same as just not having one at all, but it's not strictly
>> illegal. However, since the ACPI System Memory address space is at most
>> 64-bit, anything that would actually overflow here is already describing an
>> impossibility - really, we should probably scream even louder about a
>> firmware bug and reject it entirely, rather than quietly hiding it.
> 
> Ah, looking again I see the paths. Either acpi_dma_get_range() returns
> success, in which case base and size are fine, or it returns an EINVAL,
> in which case base=size=0, or it returns ENODEV in which case base is
> zero, so size may be set to U64_MAX by rc_dma_get_range() with no problem.
> The !dev_is_pci(dev) path is also fine since base=0.

So practically putting an explicit memory_address_limit=64 is harmless
as dmaaddr always is 0, right?

In QEMU I intended to update the ACPI code to comply with the rev D
spec. in that case the RC table revision is 1 (rev D) and the
memory_address_limit needs to be filled. If we don't want to restrict
the limit, isn't it the right choice to set 64 here?

Thanks

Eric
> 
> While I agree that we should complain when firmware provides bad ACPI
> tables, my understanding of setting iort.memory_address_limit=64 was
> that it simply meant "no limit", regardless of the base. However I now
> see that it won't be used unless base=0. So I don't think we have a
> problem, and we don't even seem to be missing firmware sanity checks.
> 
> It might be nice to have a comment explaining this stuff somewhere, but
> otherwise sorry for the noise.
> 
> Thanks,
> drew
> 
>>
>> Robin.
>>
>>> Signed-off-by: Andrew Jones 
>>> ---
>>>   drivers/acpi/arm64/iort.c | 7 ++-
>>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
>>> index 70f4e80b9246..a0f4c157ba5e 100644
>>> --- a/drivers/acpi/arm64/iort.c
>>> +++ b/drivers/acpi/arm64/iort.c
>>> @@ -1002,7 +1002,12 @@ void iort_dma_setup(struct device *dev, u64 
>>> *dma_addr, u64 *dma_size)
>>> }
>>> if (!ret) {
>>> -   msb = fls64(dmaaddr + size - 1);
>>> +   u64 dmaaddr_max = dmaaddr + size - 1;
>>> +   if (dmaaddr_max >= dmaaddr)
>>> +   msb = fls64(dmaaddr_max);
>>> +   else
>>> +   msb = 64;
>>> +
>>> /*
>>>  * Round-up to the power-of-two mask or set
>>>  * the mask to the whole 64-bit address space
>>>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

amdgpu/TTM oopses since merging swiotlb_dma_ops into the dma_direct code

[Michel Dänzer]

Hi Christoph,


https://bugs.freedesktop.org/109234 (please ignore comments #6-#9) was
bisected to your commit 55897af63091 "dma-direct: merge swiotlb_dma_ops
into the dma_direct code". Any ideas?


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu