Xen-Devel Archive mirror
 help / color / mirror / Atom feed
From: Jan Beulich <jbeulich@suse.com>
To: "Roger Pau Monné" <roger.pau@citrix.com>
Cc: xen-devel@lists.xenproject.org, julien@xen.org,
	pdurrant@amazon.com, dwmw@amazon.com,
	"Stefano Stabellini" <sstabellini@kernel.org>,
	"Bertrand Marquis" <bertrand.marquis@arm.com>,
	"Michal Orzel" <michal.orzel@amd.com>,
	"Volodymyr Babchuk" <Volodymyr_Babchuk@epam.com>,
	"Andrew Cooper" <andrew.cooper3@citrix.com>,
	"George Dunlap" <george.dunlap@citrix.com>,
	"Lukasz Hawrylko" <lukasz@hawrylko.pl>,
	"Daniel P. Smith" <dpsmith@apertussolutions.com>,
	"Mateusz Mówka" <mateusz.mowka@intel.com>,
	"Elias El Yandouzi" <eliasely@amazon.com>
Subject: Re: [PATCH V3 (resend) 14/19] Rename mfn_to_virt() calls
Date: Wed, 15 May 2024 11:42:04 +0200	[thread overview]
Message-ID: <da8f0d28-f3d5-4d64-963f-954adc7dcb45@suse.com> (raw)
In-Reply-To: <ZkSCju4RicXRuAvu@macbook>

On 15.05.2024 11:38, Roger Pau Monné wrote:
> On Tue, May 14, 2024 at 06:22:59PM +0200, Jan Beulich wrote:
>> On 14.05.2024 17:45, Roger Pau Monné wrote:
>>> On Mon, May 13, 2024 at 01:40:41PM +0000, Elias El Yandouzi wrote:
>>>> Until directmap gets completely removed, we'd still need to
>>>> keep some calls to mfn_to_virt() for xenheap pages or when the
>>>> directmap is enabled.
>>>>
>>>> Rename the macro to mfn_to_directmap_virt() to flag them and
>>>> prevent further use of mfn_to_virt().
>>>
>>> Both here and in the following patch, I'm afraid I'm unsure of it's
>>> usefulness.  I'm leaning towards this being code churn for very little
>>> benefit.
>>
>> I expect this patch is a response to an earlier comment of mine. I'm
>> rather worried that at the time this series actually goes in, un-audited
>> mfn_to_virt() uses remain (perhaps because of introduction between patch
>> submission and its committing). Such uses would all very likely end in
>> crashes or worse, but they may not be found by testing.
> 
> I see, would be good to note the intention on the commit message then.
> 
>>> Also, I'm not sure I see how the patch prevents further usage of
>>> mfn_to_virt(), as (for Arm) the existing macro is not removed.  If
>>> anything I would prefer a comment clearly stating that the macro
>>> operates on directmap space, and avoid the name change.
>>
>> But Arm isn't switched to this sparse direct map mode, I think? At which
>> point uses in Arm-specific code continue to be okay.
> 
> Right, it's just that Arm will have both mfn_to_virt() and
> mfn_to_directmap_virt() which seems a bit confusing when they are
> actually the same implementation.

Right, I agree it ends up slightly confusing. I don't think though that we
need to keep both longer term; we can likely switch back relatively soon.
We just need some "settling" period to allow people to notice and adjust
their code in whatever they have in the works.

Jan


  reply	other threads:[~2024-05-15  9:42 UTC|newest]

Thread overview: 63+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-13 13:40 [PATCH V3 (resend) 00/19] Remove the directmap Elias El Yandouzi
2024-05-13 13:40 ` [PATCH V3 (resend) 01/19] x86: Create per-domain mapping of guest_root_pt Elias El Yandouzi
2024-05-14 14:51   ` Jan Beulich
2024-05-15 18:25     ` Elias El Yandouzi
2024-05-16  7:17       ` Jan Beulich
2024-05-13 13:40 ` [PATCH V3 (resend) 02/19] x86/pv: Domheap pages should be mapped while relocating initrd Elias El Yandouzi
2024-05-13 15:40   ` Roger Pau Monné
2024-05-13 13:40 ` [PATCH V3 (resend) 03/19] x86/pv: Rewrite how building PV dom0 handles domheap mappings Elias El Yandouzi
2024-05-13 16:49   ` Roger Pau Monné
2024-05-14 14:58     ` Jan Beulich
2024-05-14 15:03   ` Jan Beulich
2024-05-13 13:40 ` [PATCH V3 (resend) 04/19] x86: Lift mapcache variable to the arch level Elias El Yandouzi
2024-05-14  8:21   ` Roger Pau Monné
2024-05-15 13:11   ` Jan Beulich
2024-05-13 13:40 ` [PATCH V3 (resend) 05/19] x86/mapcache: Initialise the mapcache for the idle domain Elias El Yandouzi
2024-05-14  8:42   ` Roger Pau Monné
2024-05-15 13:44     ` Jan Beulich
2024-05-13 13:40 ` [PATCH V3 (resend) 06/19] x86: Add a boot option to enable and disable the direct map Elias El Yandouzi
2024-05-14  9:20   ` Roger Pau Monné
2024-05-14 10:20     ` Roger Pau Monné
2024-05-15 13:54     ` Jan Beulich
2024-05-16  9:19       ` Roger Pau Monné
2024-05-16  9:24         ` Jan Beulich
2024-05-15 13:59   ` Jan Beulich
2024-05-15 16:02   ` Jan Beulich
2024-05-13 13:40 ` [PATCH V3 (resend) 07/19] xen/x86: Add support for the PMAP Elias El Yandouzi
2024-05-14  9:40   ` Roger Pau Monné
2024-05-14  9:43     ` Jan Beulich
2024-05-14 10:22       ` Roger Pau Monné
2024-05-14 10:26         ` Jan Beulich
2024-05-14 11:51           ` Roger Pau Monné
2024-05-14 12:33             ` Jan Beulich
2024-05-13 13:40 ` [PATCH V3 (resend) 08/19] xen/x86: Add build assertion for fixmap entries Elias El Yandouzi
2024-05-14  9:42   ` Roger Pau Monné
2024-05-14  9:45     ` Jan Beulich
2024-05-15 14:03   ` Jan Beulich
2024-05-13 13:40 ` [PATCH V3 (resend) 09/19] x86/domain_page: Remove the fast paths when mfn is not in the directmap Elias El Yandouzi
2024-05-14 11:48   ` Roger Pau Monné
2024-05-15 14:21   ` Jan Beulich
2024-05-13 13:40 ` [PATCH V3 (resend) 10/19] xen/page_alloc: Add a path for xenheap when there is no direct map Elias El Yandouzi
2024-05-14 13:07   ` Roger Pau Monné
2024-05-15 15:13   ` Jan Beulich
2024-05-13 13:40 ` [PATCH V3 (resend) 11/19] x86/setup: Leave early boot slightly earlier Elias El Yandouzi
2024-05-14 14:11   ` Roger Pau Monné
2024-05-15 15:22   ` Jan Beulich
2024-05-13 13:40 ` [PATCH V3 (resend) 12/19] x86/setup: vmap heap nodes when they are outside the direct map Elias El Yandouzi
2024-05-14 15:02   ` Roger Pau Monné
2024-05-15 15:28   ` Jan Beulich
2024-05-13 13:40 ` [PATCH V3 (resend) 13/19] x86/setup: Do not create valid mappings when directmap=no Elias El Yandouzi
2024-05-14 15:39   ` Roger Pau Monné
2024-05-15 15:50     ` Jan Beulich
2024-05-15 15:59   ` Jan Beulich
2024-05-13 13:40 ` [PATCH V3 (resend) 14/19] Rename mfn_to_virt() calls Elias El Yandouzi
2024-05-14 15:45   ` Roger Pau Monné
2024-05-14 16:22     ` Jan Beulich
2024-05-15  9:38       ` Roger Pau Monné
2024-05-15  9:42         ` Jan Beulich [this message]
2024-05-16  8:57   ` Jan Beulich
2024-05-13 13:40 ` [PATCH V3 (resend) 15/19] Rename maddr_to_virt() calls Elias El Yandouzi
2024-05-13 13:40 ` [PATCH V3 (resend) 16/19] xen/arm32: mm: Rename 'first' to 'root' in init_secondary_pagetables() Elias El Yandouzi
2024-05-13 13:40 ` [PATCH V3 (resend) 17/19] xen/arm64: mm: Use per-pCPU page-tables Elias El Yandouzi
2024-05-13 13:40 ` [PATCH V3 (resend) 18/19] xen/arm64: Implement a mapcache for arm64 Elias El Yandouzi
2024-05-13 13:40 ` [PATCH V3 (resend) 19/19] xen/arm64: Allow the admin to enable/disable the directmap Elias El Yandouzi

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=da8f0d28-f3d5-4d64-963f-954adc7dcb45@suse.com \
    --to=jbeulich@suse.com \
    --cc=Volodymyr_Babchuk@epam.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=bertrand.marquis@arm.com \
    --cc=dpsmith@apertussolutions.com \
    --cc=dwmw@amazon.com \
    --cc=eliasely@amazon.com \
    --cc=george.dunlap@citrix.com \
    --cc=julien@xen.org \
    --cc=lukasz@hawrylko.pl \
    --cc=mateusz.mowka@intel.com \
    --cc=michal.orzel@amd.com \
    --cc=pdurrant@amazon.com \
    --cc=roger.pau@citrix.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).