All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Thierry Escande" <thierry.escande@vates.tech>
To: "Jan Beulich" <jbeulich@suse.com>
Cc: sstabellini@kernel.org, oleksandr_tyshchenko@epam.com,
	xen-devel@lists.xenproject.org, "Juergen Gross" <jgross@suse.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] x86/xen/mmu: Increase MAX_CONTIG_ORDER
Date: Wed, 11 Dec 2024 18:20:41 +0000	[thread overview]
Message-ID: <cc61bdce-47af-45ea-8ace-173adef9ae41@vates.tech> (raw)
In-Reply-To: <ccb28ccc-531c-4ead-9a27-76cc430f8c35@suse.com>

Hi Jan,

On 09/12/2024 11:04, Jan Beulich wrote:
> On 04.12.2024 18:14, Thierry Escande wrote:
>> With change 9f40ec84a797 (xen/swiotlb: add alignment check for dma
>> buffers), the driver mpt3sas fails to load because it cannot allocate
>> its DMA pool for an allocation size of ~2,3 MBytes. This is because the
>> alignement check added by 9f40ec84a797 fails and
>> xen_swiotlb_alloc_coherent() ends up calling
>> xen_create_contiguous_region() with a size order of 10 which is too high
>> for the current max value.
>>
>> This patch increases the MAX_CONTIG_ORDER from 9 to 10 (4MB) to allow
>> such allocations.
>>
>> Signed-off-by: Thierry Escande <thierry.escande@vates.tech>
>> ---
>>  arch/x86/xen/mmu_pv.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/xen/mmu_pv.c b/arch/x86/xen/mmu_pv.c
>> index 55a4996d0c04..7f110740e1a2 100644
>> --- a/arch/x86/xen/mmu_pv.c
>> +++ b/arch/x86/xen/mmu_pv.c
>> @@ -2200,7 +2200,7 @@ void __init xen_init_mmu_ops(void)
>>  }
>>  
>>  /* Protected by xen_reservation_lock. */
>> -#define MAX_CONTIG_ORDER 9 /* 2MB */
>> +#define MAX_CONTIG_ORDER 10 /* 4MB */
>>  static unsigned long discontig_frames[1<<MAX_CONTIG_ORDER];
> 
> While lacking respective commentary, bumping this value imo also needs to
> take into account Xen itself, at least commit-message-wise. The bumping is
> fine for Dom0 in any event. It is also fine for DomU-s with the defaults
> built into the hypervisor (orders 12 and 10 respectively for x86 and Arm),
> yet especially for Arm (and in the future PPC and RISC-V) any further
> bumping would be less straightforward.

Thanks for pointing this out. On the Xen side, CONFIG_CTLDOM_MAX_ORDER
and CONFIG_HWDOM_MAX_ORDER seem big enough on all architectures. But I
see CONFIG_DOMU_MAX_ORDER set to 9 (also all archs). Won't that be a
problem for drivers trying to allocate more than that from a domU ?

> However - does the driver really need this big a contiguous chunk? It
> would seem far more desirable to me to break that up some, if possible.

Since this works on bare metal I'm afraid the driver maintainer (mpt
fusion driver) will just tell me to fix Xen.

Regards,
Thierry

> 
> Jan


WARNING: multiple messages have this Message-ID (diff)
From: "Thierry Escande" <thierry.escande@vates.tech>
To: "Jan Beulich" <jbeulich@suse.com>
Cc: sstabellini@kernel.org, oleksandr_tyshchenko@epam.com,
	xen-devel@lists.xenproject.org, "Juergen Gross" <jgross@suse.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] x86/xen/mmu: Increase MAX_CONTIG_ORDER
Date: Wed, 11 Dec 2024 18:20:42 +0000	[thread overview]
Message-ID: <cc61bdce-47af-45ea-8ace-173adef9ae41@vates.tech> (raw)
In-Reply-To: <ccb28ccc-531c-4ead-9a27-76cc430f8c35@suse.com>

Hi Jan,

On 09/12/2024 11:04, Jan Beulich wrote:
> On 04.12.2024 18:14, Thierry Escande wrote:
>> With change 9f40ec84a797 (xen/swiotlb: add alignment check for dma
>> buffers), the driver mpt3sas fails to load because it cannot allocate
>> its DMA pool for an allocation size of ~2,3 MBytes. This is because the
>> alignement check added by 9f40ec84a797 fails and
>> xen_swiotlb_alloc_coherent() ends up calling
>> xen_create_contiguous_region() with a size order of 10 which is too high
>> for the current max value.
>>
>> This patch increases the MAX_CONTIG_ORDER from 9 to 10 (4MB) to allow
>> such allocations.
>>
>> Signed-off-by: Thierry Escande <thierry.escande@vates.tech>
>> ---
>>  arch/x86/xen/mmu_pv.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/xen/mmu_pv.c b/arch/x86/xen/mmu_pv.c
>> index 55a4996d0c04..7f110740e1a2 100644
>> --- a/arch/x86/xen/mmu_pv.c
>> +++ b/arch/x86/xen/mmu_pv.c
>> @@ -2200,7 +2200,7 @@ void __init xen_init_mmu_ops(void)
>>  }
>>  
>>  /* Protected by xen_reservation_lock. */
>> -#define MAX_CONTIG_ORDER 9 /* 2MB */
>> +#define MAX_CONTIG_ORDER 10 /* 4MB */
>>  static unsigned long discontig_frames[1<<MAX_CONTIG_ORDER];
> 
> While lacking respective commentary, bumping this value imo also needs to
> take into account Xen itself, at least commit-message-wise. The bumping is
> fine for Dom0 in any event. It is also fine for DomU-s with the defaults
> built into the hypervisor (orders 12 and 10 respectively for x86 and Arm),
> yet especially for Arm (and in the future PPC and RISC-V) any further
> bumping would be less straightforward.

Thanks for pointing this out. On the Xen side, CONFIG_CTLDOM_MAX_ORDER
and CONFIG_HWDOM_MAX_ORDER seem big enough on all architectures. But I
see CONFIG_DOMU_MAX_ORDER set to 9 (also all archs). Won't that be a
problem for drivers trying to allocate more than that from a domU ?

> However - does the driver really need this big a contiguous chunk? It
> would seem far more desirable to me to break that up some, if possible.

Since this works on bare metal I'm afraid the driver maintainer (mpt
fusion driver) will just tell me to fix Xen.

Regards,
Thierry

> 
> Jan

  reply	other threads:[~2024-12-11 18:21 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-04 17:14 [PATCH] x86/xen/mmu: Increase MAX_CONTIG_ORDER Thierry Escande
2024-12-09 10:04 ` Jan Beulich
2024-12-11 18:20   ` Thierry Escande [this message]
2024-12-11 18:20     ` Thierry Escande
2024-12-12 10:22     ` Jan Beulich
2024-12-12 11:09       ` Juergen Gross
2024-12-18 11:11         ` Thierry Escande
2024-12-18 11:24           ` Jürgen Groß
2024-12-19  7:12             ` Jan Beulich
2025-01-04 18:50               ` Jürgen Groß

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cc61bdce-47af-45ea-8ace-173adef9ae41@vates.tech \
    --to=thierry.escande@vates.tech \
    --cc=jbeulich@suse.com \
    --cc=jgross@suse.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleksandr_tyshchenko@epam.com \
    --cc=sstabellini@kernel.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.