xen/arch/arm/Kconfig | 1 - xen/arch/arm/include/asm/mm.h | 3 +- xen/arch/x86/Kconfig | 1 - xen/arch/x86/domain.c | 19 ++-- xen/arch/x86/include/asm/x86_64/page.h | 28 +++--- xen/arch/x86/x86_64/mm.c | 6 +- xen/common/Kconfig | 13 ++- xen/common/Makefile | 2 +- xen/common/efi/boot.c | 13 ++- xen/common/pdx.c | 76 +++++++++------ xen/include/xen/pdx.h | 127 +++++++++++++++++++------ 11 files changed, 193 insertions(+), 96 deletions(-)
Currently there's a CONFIG_HAS_PDX Kconfig option, but it's impossible to
disable it because the whole codebase performs unconditional
compression/decompression operations on addresses. This has the
unfortunate side effect that systems without a need for compression still
have to pay the performance impact of juggling bits on every pfn<->pdx
conversion (this requires reading several global variables). This series
attempts to:
* Leave the state of pdx and pdx compression documented
* Factor out compression so it _can_ be removed through Kconfig
* Make it so compression is disabled on x86 and enabled on both Aarch32
and Aarch64 by default.
Series summary:
Patch 1 Moves hard-coded compression-related logic to helper functions
Patch 2 Refactors all instances of regions being validated for pdx
compression conformance so it's done through a helper
Patch 3 Non-functional reorder in order to simplify the patch 8 diff
Patch 4 Adds new Kconfig option to compile out PDX compression and removes
the old CONFIG_HAS_PDX, as it was non removable
Already committed:
v1/patch 1 documents the current general understanding of the pdx concept and
pdx compression in particular
v1/patch 3 Marks the pdx compression globals as ro_after_init
v2/patch 1 Documents the differences between arm32 and arm64 directmaps
Alejandro Vallejo (4):
mm: Factor out the pdx compression logic in ma/va converters
mm/pdx: Standardize region validation wrt pdx compression
pdx: Reorder pdx.[ch]
pdx: Add CONFIG_PDX_COMPRESSION as a common Kconfig option
xen/arch/arm/Kconfig | 1 -
xen/arch/arm/include/asm/mm.h | 3 +-
xen/arch/x86/Kconfig | 1 -
xen/arch/x86/domain.c | 19 ++--
xen/arch/x86/include/asm/x86_64/page.h | 28 +++---
xen/arch/x86/x86_64/mm.c | 6 +-
xen/common/Kconfig | 13 ++-
xen/common/Makefile | 2 +-
xen/common/efi/boot.c | 13 ++-
xen/common/pdx.c | 76 +++++++++------
xen/include/xen/pdx.h | 127 +++++++++++++++++++------
11 files changed, 193 insertions(+), 96 deletions(-)
--
2.34.1
On Tue, Aug 08, 2023 at 02:02:16PM +0100, Alejandro Vallejo wrote: > Currently there's a CONFIG_HAS_PDX Kconfig option, but it's impossible to > disable it because the whole codebase performs unconditional > compression/decompression operations on addresses. This has the > unfortunate side effect that systems without a need for compression still > have to pay the performance impact of juggling bits on every pfn<->pdx > conversion (this requires reading several global variables). This series > attempts to: > > * Leave the state of pdx and pdx compression documented > * Factor out compression so it _can_ be removed through Kconfig > * Make it so compression is disabled on x86 and enabled on both Aarch32 > and Aarch64 by default. > > Series summary: > > Patch 1 Moves hard-coded compression-related logic to helper functions > Patch 2 Refactors all instances of regions being validated for pdx > compression conformance so it's done through a helper > Patch 3 Non-functional reorder in order to simplify the patch 8 diff > Patch 4 Adds new Kconfig option to compile out PDX compression and removes > the old CONFIG_HAS_PDX, as it was non removable > > Already committed: > > v1/patch 1 documents the current general understanding of the pdx concept and > pdx compression in particular > v1/patch 3 Marks the pdx compression globals as ro_after_init > v2/patch 1 Documents the differences between arm32 and arm64 directmaps > > Alejandro Vallejo (4): > mm: Factor out the pdx compression logic in ma/va converters > mm/pdx: Standardize region validation wrt pdx compression > pdx: Reorder pdx.[ch] > pdx: Add CONFIG_PDX_COMPRESSION as a common Kconfig option @Jan: Just making sure, are you generally ok with this series as-is? Thanks, Alejandro
On 16.08.2023 11:36, Alejandro Vallejo wrote: > On Tue, Aug 08, 2023 at 02:02:16PM +0100, Alejandro Vallejo wrote: >> Currently there's a CONFIG_HAS_PDX Kconfig option, but it's impossible to >> disable it because the whole codebase performs unconditional >> compression/decompression operations on addresses. This has the >> unfortunate side effect that systems without a need for compression still >> have to pay the performance impact of juggling bits on every pfn<->pdx >> conversion (this requires reading several global variables). This series >> attempts to: >> >> * Leave the state of pdx and pdx compression documented >> * Factor out compression so it _can_ be removed through Kconfig >> * Make it so compression is disabled on x86 and enabled on both Aarch32 >> and Aarch64 by default. >> >> Series summary: >> >> Patch 1 Moves hard-coded compression-related logic to helper functions >> Patch 2 Refactors all instances of regions being validated for pdx >> compression conformance so it's done through a helper >> Patch 3 Non-functional reorder in order to simplify the patch 8 diff >> Patch 4 Adds new Kconfig option to compile out PDX compression and removes >> the old CONFIG_HAS_PDX, as it was non removable >> >> Already committed: >> >> v1/patch 1 documents the current general understanding of the pdx concept and >> pdx compression in particular >> v1/patch 3 Marks the pdx compression globals as ro_after_init >> v2/patch 1 Documents the differences between arm32 and arm64 directmaps >> >> Alejandro Vallejo (4): >> mm: Factor out the pdx compression logic in ma/va converters >> mm/pdx: Standardize region validation wrt pdx compression >> pdx: Reorder pdx.[ch] >> pdx: Add CONFIG_PDX_COMPRESSION as a common Kconfig option > > @Jan: Just making sure, are you generally ok with this series as-is? Well, okay would be too strong; I still don't see why my runtime patching series isn't re-considered. But I don't mind it going in the way it is now. I won't ack any part of it, though (in case that wasn't obvious), so it'll be up to Andrew or Roger to supply the necessary x86 acks. Jan
Hi Jan, On 16/08/2023 10:43, Jan Beulich wrote: > On 16.08.2023 11:36, Alejandro Vallejo wrote: >> On Tue, Aug 08, 2023 at 02:02:16PM +0100, Alejandro Vallejo wrote: >>> Currently there's a CONFIG_HAS_PDX Kconfig option, but it's impossible to >>> disable it because the whole codebase performs unconditional >>> compression/decompression operations on addresses. This has the >>> unfortunate side effect that systems without a need for compression still >>> have to pay the performance impact of juggling bits on every pfn<->pdx >>> conversion (this requires reading several global variables). This series >>> attempts to: >>> >>> * Leave the state of pdx and pdx compression documented >>> * Factor out compression so it _can_ be removed through Kconfig >>> * Make it so compression is disabled on x86 and enabled on both Aarch32 >>> and Aarch64 by default. >>> >>> Series summary: >>> >>> Patch 1 Moves hard-coded compression-related logic to helper functions >>> Patch 2 Refactors all instances of regions being validated for pdx >>> compression conformance so it's done through a helper >>> Patch 3 Non-functional reorder in order to simplify the patch 8 diff >>> Patch 4 Adds new Kconfig option to compile out PDX compression and removes >>> the old CONFIG_HAS_PDX, as it was non removable >>> >>> Already committed: >>> >>> v1/patch 1 documents the current general understanding of the pdx concept and >>> pdx compression in particular >>> v1/patch 3 Marks the pdx compression globals as ro_after_init >>> v2/patch 1 Documents the differences between arm32 and arm64 directmaps >>> >>> Alejandro Vallejo (4): >>> mm: Factor out the pdx compression logic in ma/va converters >>> mm/pdx: Standardize region validation wrt pdx compression >>> pdx: Reorder pdx.[ch] >>> pdx: Add CONFIG_PDX_COMPRESSION as a common Kconfig option >> >> @Jan: Just making sure, are you generally ok with this series as-is? > > Well, okay would be too strong; I still don't see why my runtime patching > series isn't re-considered. Do you have a pointer to the series? I would be interested to have a look. That said... the problem with alt-patching is this is architectural specific. Right now, this seems to be a bit unnecessary given that we believe that virtually no-one will have a platform (I know we talked about a potential one...) where PDX is compressing. Cheers, -- Julien Grall
On 16.08.2023 13:12, Julien Grall wrote: > Hi Jan, > > On 16/08/2023 10:43, Jan Beulich wrote: >> On 16.08.2023 11:36, Alejandro Vallejo wrote: >>> On Tue, Aug 08, 2023 at 02:02:16PM +0100, Alejandro Vallejo wrote: >>>> Currently there's a CONFIG_HAS_PDX Kconfig option, but it's impossible to >>>> disable it because the whole codebase performs unconditional >>>> compression/decompression operations on addresses. This has the >>>> unfortunate side effect that systems without a need for compression still >>>> have to pay the performance impact of juggling bits on every pfn<->pdx >>>> conversion (this requires reading several global variables). This series >>>> attempts to: >>>> >>>> * Leave the state of pdx and pdx compression documented >>>> * Factor out compression so it _can_ be removed through Kconfig >>>> * Make it so compression is disabled on x86 and enabled on both Aarch32 >>>> and Aarch64 by default. >>>> >>>> Series summary: >>>> >>>> Patch 1 Moves hard-coded compression-related logic to helper functions >>>> Patch 2 Refactors all instances of regions being validated for pdx >>>> compression conformance so it's done through a helper >>>> Patch 3 Non-functional reorder in order to simplify the patch 8 diff >>>> Patch 4 Adds new Kconfig option to compile out PDX compression and removes >>>> the old CONFIG_HAS_PDX, as it was non removable >>>> >>>> Already committed: >>>> >>>> v1/patch 1 documents the current general understanding of the pdx concept and >>>> pdx compression in particular >>>> v1/patch 3 Marks the pdx compression globals as ro_after_init >>>> v2/patch 1 Documents the differences between arm32 and arm64 directmaps >>>> >>>> Alejandro Vallejo (4): >>>> mm: Factor out the pdx compression logic in ma/va converters >>>> mm/pdx: Standardize region validation wrt pdx compression >>>> pdx: Reorder pdx.[ch] >>>> pdx: Add CONFIG_PDX_COMPRESSION as a common Kconfig option >>> >>> @Jan: Just making sure, are you generally ok with this series as-is? >> >> Well, okay would be too strong; I still don't see why my runtime patching >> series isn't re-considered. > > Do you have a pointer to the series? I would be interested to have a look. Sure, I can dig it out a 2nd time: https://lists.xenproject.org/archives/html/xen-devel/2018-09/msg01616.html > That said... the problem with alt-patching is this is architectural > specific. Right now, this seems to be a bit unnecessary given that we > believe that virtually no-one will have a platform (I know we talked > about a potential one...) where PDX is compressing. But it defaults to enabled on other than x86 anyway. So it seems like it's generally wanted everywhere except on x86, and on x86 it can (could) be patched out. Jan
Hi, On 16/08/2023 12:27, Jan Beulich wrote: > On 16.08.2023 13:12, Julien Grall wrote: >> Hi Jan, >> >> On 16/08/2023 10:43, Jan Beulich wrote: >>> On 16.08.2023 11:36, Alejandro Vallejo wrote: >>>> On Tue, Aug 08, 2023 at 02:02:16PM +0100, Alejandro Vallejo wrote: >>>>> Currently there's a CONFIG_HAS_PDX Kconfig option, but it's impossible to >>>>> disable it because the whole codebase performs unconditional >>>>> compression/decompression operations on addresses. This has the >>>>> unfortunate side effect that systems without a need for compression still >>>>> have to pay the performance impact of juggling bits on every pfn<->pdx >>>>> conversion (this requires reading several global variables). This series >>>>> attempts to: >>>>> >>>>> * Leave the state of pdx and pdx compression documented >>>>> * Factor out compression so it _can_ be removed through Kconfig >>>>> * Make it so compression is disabled on x86 and enabled on both Aarch32 >>>>> and Aarch64 by default. >>>>> >>>>> Series summary: >>>>> >>>>> Patch 1 Moves hard-coded compression-related logic to helper functions >>>>> Patch 2 Refactors all instances of regions being validated for pdx >>>>> compression conformance so it's done through a helper >>>>> Patch 3 Non-functional reorder in order to simplify the patch 8 diff >>>>> Patch 4 Adds new Kconfig option to compile out PDX compression and removes >>>>> the old CONFIG_HAS_PDX, as it was non removable >>>>> >>>>> Already committed: >>>>> >>>>> v1/patch 1 documents the current general understanding of the pdx concept and >>>>> pdx compression in particular >>>>> v1/patch 3 Marks the pdx compression globals as ro_after_init >>>>> v2/patch 1 Documents the differences between arm32 and arm64 directmaps >>>>> >>>>> Alejandro Vallejo (4): >>>>> mm: Factor out the pdx compression logic in ma/va converters >>>>> mm/pdx: Standardize region validation wrt pdx compression >>>>> pdx: Reorder pdx.[ch] >>>>> pdx: Add CONFIG_PDX_COMPRESSION as a common Kconfig option >>>> >>>> @Jan: Just making sure, are you generally ok with this series as-is? >>> >>> Well, okay would be too strong; I still don't see why my runtime patching >>> series isn't re-considered. >> >> Do you have a pointer to the series? I would be interested to have a look. > > Sure, I can dig it out a 2nd time: > https://lists.xenproject.org/archives/html/xen-devel/2018-09/msg01616.html Thanks! AFAIU, the optimization would only happen after the alternative has been patched. This is happening after initializing the heap. With Alejandro series, you will have some performance gain for the boot which I care about. > >> That said... the problem with alt-patching is this is architectural >> specific. Right now, this seems to be a bit unnecessary given that we >> believe that virtually no-one will have a platform (I know we talked >> about a potential one...) where PDX is compressing. > > But it defaults to enabled on other than x86 anyway. So it seems like > it's generally wanted everywhere except on x86, and on x86 it can > (could) be patched out. IIUC, you are saying that we would want both Alejandro series (to allow an admin to disable PDX at boot) and your series (to fully optimize the PDX at runtime). Is that correct? If so, it is unclear to me why you want your series to be re-considered now. Can't this be done as a follow-up if there is a desire to further optimize? Cheers, -- Julien Grall
On 16.08.2023 15:06, Julien Grall wrote: > Hi, > > On 16/08/2023 12:27, Jan Beulich wrote: >> On 16.08.2023 13:12, Julien Grall wrote: >>> Hi Jan, >>> >>> On 16/08/2023 10:43, Jan Beulich wrote: >>>> On 16.08.2023 11:36, Alejandro Vallejo wrote: >>>>> On Tue, Aug 08, 2023 at 02:02:16PM +0100, Alejandro Vallejo wrote: >>>>>> Currently there's a CONFIG_HAS_PDX Kconfig option, but it's impossible to >>>>>> disable it because the whole codebase performs unconditional >>>>>> compression/decompression operations on addresses. This has the >>>>>> unfortunate side effect that systems without a need for compression still >>>>>> have to pay the performance impact of juggling bits on every pfn<->pdx >>>>>> conversion (this requires reading several global variables). This series >>>>>> attempts to: >>>>>> >>>>>> * Leave the state of pdx and pdx compression documented >>>>>> * Factor out compression so it _can_ be removed through Kconfig >>>>>> * Make it so compression is disabled on x86 and enabled on both Aarch32 >>>>>> and Aarch64 by default. >>>>>> >>>>>> Series summary: >>>>>> >>>>>> Patch 1 Moves hard-coded compression-related logic to helper functions >>>>>> Patch 2 Refactors all instances of regions being validated for pdx >>>>>> compression conformance so it's done through a helper >>>>>> Patch 3 Non-functional reorder in order to simplify the patch 8 diff >>>>>> Patch 4 Adds new Kconfig option to compile out PDX compression and removes >>>>>> the old CONFIG_HAS_PDX, as it was non removable >>>>>> >>>>>> Already committed: >>>>>> >>>>>> v1/patch 1 documents the current general understanding of the pdx concept and >>>>>> pdx compression in particular >>>>>> v1/patch 3 Marks the pdx compression globals as ro_after_init >>>>>> v2/patch 1 Documents the differences between arm32 and arm64 directmaps >>>>>> >>>>>> Alejandro Vallejo (4): >>>>>> mm: Factor out the pdx compression logic in ma/va converters >>>>>> mm/pdx: Standardize region validation wrt pdx compression >>>>>> pdx: Reorder pdx.[ch] >>>>>> pdx: Add CONFIG_PDX_COMPRESSION as a common Kconfig option >>>>> >>>>> @Jan: Just making sure, are you generally ok with this series as-is? >>>> >>>> Well, okay would be too strong; I still don't see why my runtime patching >>>> series isn't re-considered. >>> >>> Do you have a pointer to the series? I would be interested to have a look. >> >> Sure, I can dig it out a 2nd time: >> https://lists.xenproject.org/archives/html/xen-devel/2018-09/msg01616.html > > Thanks! AFAIU, the optimization would only happen after the alternative > has been patched. This is happening after initializing the heap. With > Alejandro series, you will have some performance gain for the boot which > I care about. Fair aspect. >>> That said... the problem with alt-patching is this is architectural >>> specific. Right now, this seems to be a bit unnecessary given that we >>> believe that virtually no-one will have a platform (I know we talked >>> about a potential one...) where PDX is compressing. >> >> But it defaults to enabled on other than x86 anyway. So it seems like >> it's generally wanted everywhere except on x86, and on x86 it can >> (could) be patched out. > > IIUC, you are saying that we would want both Alejandro series (to allow > an admin to disable PDX at boot) and your series (to fully optimize the > PDX at runtime). Is that correct? Not really, no. I don't view a build-time decision as necessary; I favor runtime decisions whenever possible. But as said I also don't mind this series going in. > If so, it is unclear to me why you want your series to be re-considered > now. Can't this be done as a follow-up if there is a desire to further > optimize? In principle it could be, yes, but I'm afraid I know that no follow-up is going to happen (and me trying to revive the earlier work would be yet another case of pointlessly invested time). Jan
Hi, On 16/08/2023 14:14, Jan Beulich wrote: > On 16.08.2023 15:06, Julien Grall wrote: >> Hi, >> >> On 16/08/2023 12:27, Jan Beulich wrote: >>> On 16.08.2023 13:12, Julien Grall wrote: >>>> Hi Jan, >>>> >>>> On 16/08/2023 10:43, Jan Beulich wrote: >>>>> On 16.08.2023 11:36, Alejandro Vallejo wrote: >>>>>> On Tue, Aug 08, 2023 at 02:02:16PM +0100, Alejandro Vallejo wrote: >>>>>>> Currently there's a CONFIG_HAS_PDX Kconfig option, but it's impossible to >>>>>>> disable it because the whole codebase performs unconditional >>>>>>> compression/decompression operations on addresses. This has the >>>>>>> unfortunate side effect that systems without a need for compression still >>>>>>> have to pay the performance impact of juggling bits on every pfn<->pdx >>>>>>> conversion (this requires reading several global variables). This series >>>>>>> attempts to: >>>>>>> >>>>>>> * Leave the state of pdx and pdx compression documented >>>>>>> * Factor out compression so it _can_ be removed through Kconfig >>>>>>> * Make it so compression is disabled on x86 and enabled on both Aarch32 >>>>>>> and Aarch64 by default. >>>>>>> >>>>>>> Series summary: >>>>>>> >>>>>>> Patch 1 Moves hard-coded compression-related logic to helper functions >>>>>>> Patch 2 Refactors all instances of regions being validated for pdx >>>>>>> compression conformance so it's done through a helper >>>>>>> Patch 3 Non-functional reorder in order to simplify the patch 8 diff >>>>>>> Patch 4 Adds new Kconfig option to compile out PDX compression and removes >>>>>>> the old CONFIG_HAS_PDX, as it was non removable >>>>>>> >>>>>>> Already committed: >>>>>>> >>>>>>> v1/patch 1 documents the current general understanding of the pdx concept and >>>>>>> pdx compression in particular >>>>>>> v1/patch 3 Marks the pdx compression globals as ro_after_init >>>>>>> v2/patch 1 Documents the differences between arm32 and arm64 directmaps >>>>>>> >>>>>>> Alejandro Vallejo (4): >>>>>>> mm: Factor out the pdx compression logic in ma/va converters >>>>>>> mm/pdx: Standardize region validation wrt pdx compression >>>>>>> pdx: Reorder pdx.[ch] >>>>>>> pdx: Add CONFIG_PDX_COMPRESSION as a common Kconfig option >>>>>> >>>>>> @Jan: Just making sure, are you generally ok with this series as-is? >>>>> >>>>> Well, okay would be too strong; I still don't see why my runtime patching >>>>> series isn't re-considered. >>>> >>>> Do you have a pointer to the series? I would be interested to have a look. >>> >>> Sure, I can dig it out a 2nd time: >>> https://lists.xenproject.org/archives/html/xen-devel/2018-09/msg01616.html >> >> Thanks! AFAIU, the optimization would only happen after the alternative >> has been patched. This is happening after initializing the heap. With >> Alejandro series, you will have some performance gain for the boot which >> I care about. > > Fair aspect. > >>>> That said... the problem with alt-patching is this is architectural >>>> specific. Right now, this seems to be a bit unnecessary given that we >>>> believe that virtually no-one will have a platform (I know we talked >>>> about a potential one...) where PDX is compressing. >>> >>> But it defaults to enabled on other than x86 anyway. So it seems like >>> it's generally wanted everywhere except on x86, and on x86 it can >>> (could) be patched out. >> >> IIUC, you are saying that we would want both Alejandro series (to allow >> an admin to disable PDX at boot) and your series (to fully optimize the >> PDX at runtime). Is that correct? > > Not really, no. I don't view a build-time decision as necessary; I favor > runtime decisions whenever possible. At least on Arm, we want to cater two different set of users. One set will want to tailor Xen to there platform and runtime decision may not be desirable. The other set (e.g. distros) will want to run Xen everywhere so runtime is preferable. So I think we should be able to offer both build and runtime option when it makes sense. The PDX is one example where both could be interesting, yet at the moment there seem to be an appetite for build time only configuration. > >> If so, it is unclear to me why you want your series to be re-considered >> now. Can't this be done as a follow-up if there is a desire to further >> optimize? > > In principle it could be, yes, but I'm afraid I know that no follow-up > is going to happen (and me trying to revive the earlier work would be > yet another case of pointlessly invested time). Right which is why I wrote "desire". But my point was that Alejandro work is not a one-way door. It would still possible to have runtime patching on top of it. Cheers, -- Julien Grall
© 2016 - 2024 Red Hat, Inc.