All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Beulich <jbeulich@suse.com>
To: Julien Grall <julien@xen.org>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	Volodymyr Babchuk <volodymyr_babchuk@epam.com>,
	Bertrand Marquis <bertrand.marquis@arm.com>,
	Michal Orzel <michal.orzel@amd.com>,
	Oleksii Kurochko <oleksii.kurochko@gmail.com>,
	"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH] Arm: drop assertion from page_is_ram_type()
Date: Tue, 12 Aug 2025 08:29:50 +0200	[thread overview]
Message-ID: <ece155ab-d247-41cb-b3a1-ae4233423cef@suse.com> (raw)
In-Reply-To: <86faa2fa-9f23-49a9-a909-a3dd3d5a938f@xen.org>

On 11.08.2025 19:24, Julien Grall wrote:
> On 11/08/2025 14:01, Jan Beulich wrote:
>> Its uses in offline_page() and query_page_offline() make it reachable on
>> Arm, as long as XEN_SYSCTL_page_offline_op doesn't have any Arm-specific
>> code added. It being reachable was even mentioned in the commit
>> introducing it, claiming it "clearly shouldn't be called on ARM just
>> yet".
> 
> So I agree that the function can be reached. But then I don't think the 
> function can simply return 0.
> 
> The name is too generic enough that someone may end up to use it in 
> common code and there will be no signal to the user that the function 
> will not properly indicate a RAM page on Arm.
> 
> I can think of two possible approaches:
> 
> 1/ Implement properly page_is_ram_type(). We don't have an e820, but we 
> could mimick it using the memory banks we stored.
> 2/ Rename page_is_ram_type() to page_offlinable() (or similar) so it is 
> clear that the common use is for offlining.
> 
> The latter might be the simplest.

I consider page_offlinable() potentially ambiguous: When you offline a PCI
device, you offline its MMIO as well, for example. Or, much like RAM, some
page in, say, a video device's framebuffer may have gone bad. One might
think that such would then also be covered by that function. So minimally
page_offlinable_ram() or page_is_offlinable_ram(), I think. That would
then have the benefit of allowing an avenue towards x86 also making its
checking more precise, as certain RAM ranges can't possibly be offlined.
If that's fine with you, I can enlarge the patch accordingly.

Jan


  reply	other threads:[~2025-08-12  6:30 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-11 13:01 [PATCH] Arm: drop assertion from page_is_ram_type() Jan Beulich
2025-08-11 17:24 ` Julien Grall
2025-08-12  6:29   ` Jan Beulich [this message]
2025-08-12 11:05     ` Julien Grall

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=ece155ab-d247-41cb-b3a1-ae4233423cef@suse.com \
    --to=jbeulich@suse.com \
    --cc=bertrand.marquis@arm.com \
    --cc=julien@xen.org \
    --cc=michal.orzel@amd.com \
    --cc=oleksii.kurochko@gmail.com \
    --cc=sstabellini@kernel.org \
    --cc=volodymyr_babchuk@epam.com \
    --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.