Re: [PATCH] iommu/amd: Enforce 4k mapping for certain IOMMU data structures
Will,
I have already submitted v2 of this patch. Let me move the discussion there
instead ...
(https://lore.kernel.org/linux-iommu/20201105145832.3065-1-suravee.suthikulpa...@amd.com/)
Suravee
On 11/18/20 5:57 AM, Will Deacon wrote:
On Wed, Oct 28, 2020 at 11:18:24PM +, Suravee Suthikulpanit wrote:
AMD IOMMU requires 4k-aligned pages for the event log, the PPR log,
and the completion wait write-back regions. However, when allocating
the pages, they could be part of large mapping (e.g. 2M) page.
This causes #PF due to the SNP RMP hardware enforces the check based
on the page level for these data structures.
Please could you include an example backtrace here?
So, fix by calling set_memory_4k() on the allocated pages.
I think I'm missing something here. set_memory_4k() will break the kernel
linear mapping up into page granular mappings, but the IOMMU isn't using
that mapping, right? It's just using the physical address returned by
iommu_virt_to_phys(), so why does it matter?
Just be nice to capture some of this rationale in the log, especially as
I'm not familiar with this device.
Fixes: commit c69d89aff393 ("iommu/amd: Use 4K page for completion wait write-back
semaphore")
I couldn't figure out how that commit could cause this problem. Please can
you explain that to me?
Cheers,
Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/amd: Enforce 4k mapping for certain IOMMU data structures
On Wed, Oct 28, 2020 at 11:18:24PM +, Suravee Suthikulpanit wrote:
> AMD IOMMU requires 4k-aligned pages for the event log, the PPR log,
> and the completion wait write-back regions. However, when allocating
> the pages, they could be part of large mapping (e.g. 2M) page.
> This causes #PF due to the SNP RMP hardware enforces the check based
> on the page level for these data structures.
Please could you include an example backtrace here?
> So, fix by calling set_memory_4k() on the allocated pages.
I think I'm missing something here. set_memory_4k() will break the kernel
linear mapping up into page granular mappings, but the IOMMU isn't using
that mapping, right? It's just using the physical address returned by
iommu_virt_to_phys(), so why does it matter?
Just be nice to capture some of this rationale in the log, especially as
I'm not familiar with this device.
> Fixes: commit c69d89aff393 ("iommu/amd: Use 4K page for completion wait
> write-back semaphore")
I couldn't figure out how that commit could cause this problem. Please can
you explain that to me?
Cheers,
Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/amd: Enforce 4k mapping for certain IOMMU data structures
Hi Suravee,
On Wed, Oct 28, 2020 at 11:18:24PM +, Suravee Suthikulpanit wrote:
> AMD IOMMU requires 4k-aligned pages for the event log, the PPR log,
> and the completion wait write-back regions. However, when allocating
> the pages, they could be part of large mapping (e.g. 2M) page.
> This causes #PF due to the SNP RMP hardware enforces the check based
> on the page level for these data structures.
>
> So, fix by calling set_memory_4k() on the allocated pages.
>
> Fixes: commit c69d89aff393 ("iommu/amd: Use 4K page for completion wait
> write-back semaphore")
> Cc: Brijesh Singh
> Signed-off-by: Suravee Suthikulpanit
> ---
> drivers/iommu/amd/init.c | 22 +-
> 1 file changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> index 82e4af8f09bb..75dc30226a7c 100644
> --- a/drivers/iommu/amd/init.c
> +++ b/drivers/iommu/amd/init.c
> @@ -29,6 +29,7 @@
> #include
> #include
> #include
> +#include
>
> #include
>
> @@ -672,11 +673,22 @@ static void __init free_command_buffer(struct amd_iommu
> *iommu)
> free_pages((unsigned long)iommu->cmd_buf, get_order(CMD_BUFFER_SIZE));
> }
>
> +static void *__init iommu_alloc_4k_pages(gfp_t gfp, size_t size)
> +{
> + void *buf;
> + int order = get_order(size);
> +
> + buf = (void *)__get_free_pages(gfp, order);
> + if (!buf)
> + return buf;
> + return set_memory_4k((unsigned long)buf, (1 << order)) ? NULL : buf;
> +}
> +
Please make the 4k split only if SNP is actually enabled in the system.
Regards,
Joerg
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu/amd: Enforce 4k mapping for certain IOMMU data structures
AMD IOMMU requires 4k-aligned pages for the event log, the PPR log,
and the completion wait write-back regions. However, when allocating
the pages, they could be part of large mapping (e.g. 2M) page.
This causes #PF due to the SNP RMP hardware enforces the check based
on the page level for these data structures.
So, fix by calling set_memory_4k() on the allocated pages.
Fixes: commit c69d89aff393 ("iommu/amd: Use 4K page for completion wait
write-back semaphore")
Cc: Brijesh Singh
Signed-off-by: Suravee Suthikulpanit
---
drivers/iommu/amd/init.c | 22 +-
1 file changed, 17 insertions(+), 5 deletions(-)
diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index 82e4af8f09bb..75dc30226a7c 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -29,6 +29,7 @@
#include
#include
#include
+#include
#include
@@ -672,11 +673,22 @@ static void __init free_command_buffer(struct amd_iommu
*iommu)
free_pages((unsigned long)iommu->cmd_buf, get_order(CMD_BUFFER_SIZE));
}
+static void *__init iommu_alloc_4k_pages(gfp_t gfp, size_t size)
+{
+ void *buf;
+ int order = get_order(size);
+
+ buf = (void *)__get_free_pages(gfp, order);
+ if (!buf)
+ return buf;
+ return set_memory_4k((unsigned long)buf, (1 << order)) ? NULL : buf;
+}
+
/* allocates the memory where the IOMMU will log its events to */
static int __init alloc_event_buffer(struct amd_iommu *iommu)
{
- iommu->evt_buf = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO,
- get_order(EVT_BUFFER_SIZE));
+ iommu->evt_buf = iommu_alloc_4k_pages(GFP_KERNEL | __GFP_ZERO,
+ EVT_BUFFER_SIZE);
return iommu->evt_buf ? 0 : -ENOMEM;
}
@@ -715,8 +727,8 @@ static void __init free_event_buffer(struct amd_iommu
*iommu)
/* allocates the memory where the IOMMU will log its events to */
static int __init alloc_ppr_log(struct amd_iommu *iommu)
{
- iommu->ppr_log = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO,
- get_order(PPR_LOG_SIZE));
+ iommu->ppr_log = iommu_alloc_4k_pages(GFP_KERNEL | __GFP_ZERO,
+ PPR_LOG_SIZE);
return iommu->ppr_log ? 0 : -ENOMEM;
}
@@ -838,7 +850,7 @@ static int iommu_init_ga(struct amd_iommu *iommu)
static int __init alloc_cwwb_sem(struct amd_iommu *iommu)
{
- iommu->cmd_sem = (void *)get_zeroed_page(GFP_KERNEL);
+ iommu->cmd_sem = iommu_alloc_4k_pages(GFP_KERNEL | __GFP_ZERO, 1);
return iommu->cmd_sem ? 0 : -ENOMEM;
}
--
2.17.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
