Messages in this thread Patch in this message | | | Date | Thu, 11 Apr 2024 12:38:17 +0200 | Subject | Re: [PATCH v1 3/5] s390/uv: convert PG_arch_1 users to only work on small folios | From | David Hildenbrand <> |
| |
On 10.04.24 19:42, Claudio Imbrenda wrote: > On Thu, 4 Apr 2024 18:36:40 +0200 > David Hildenbrand <david@redhat.com> wrote: > >> Now that make_folio_secure() may only set PG_arch_1 for small folios, >> let's convert relevant remaining UV code to only work on (small) folios >> and simply reject large folios early. This way, we'll never end up >> touching PG_arch_1 on tail pages of a large folio in UV code. >> >> The folio_get()/folio_put() for functions that are documented to already >> hold a folio reference look weird and it should probably be removed. >> Similarly, uv_destroy_owned_page() and uv_convert_owned_from_secure() >> should really consume a folio reference instead. But these are cleanups for >> another day. >> >> Signed-off-by: David Hildenbrand <david@redhat.com> >> --- >> arch/s390/include/asm/page.h | 1 + >> arch/s390/kernel/uv.c | 39 +++++++++++++++++++++--------------- >> 2 files changed, 24 insertions(+), 16 deletions(-) >> >> diff --git a/arch/s390/include/asm/page.h b/arch/s390/include/asm/page.h >> index 54d015bcd8e3..b64384872c0f 100644 >> --- a/arch/s390/include/asm/page.h >> +++ b/arch/s390/include/asm/page.h >> @@ -214,6 +214,7 @@ static inline unsigned long __phys_addr(unsigned long x, bool is_31bit) >> #define pfn_to_phys(pfn) ((pfn) << PAGE_SHIFT) >> >> #define phys_to_page(phys) pfn_to_page(phys_to_pfn(phys)) >> +#define phys_to_folio(phys) page_folio(phys_to_page(phys)) >> #define page_to_phys(page) pfn_to_phys(page_to_pfn(page)) >> #define folio_to_phys(page) pfn_to_phys(folio_pfn(folio)) >> >> diff --git a/arch/s390/kernel/uv.c b/arch/s390/kernel/uv.c >> index adcbd4b13035..9c0113b26735 100644 >> --- a/arch/s390/kernel/uv.c >> +++ b/arch/s390/kernel/uv.c >> @@ -134,14 +134,17 @@ static int uv_destroy_page(unsigned long paddr) >> */ >> int uv_destroy_owned_page(unsigned long paddr) >> { >> - struct page *page = phys_to_page(paddr); >> + struct folio *folio = phys_to_folio(paddr); >> int rc; >> >> - get_page(page); >> + if (unlikely(folio_test_large(folio))) >> + return 0; > > please add a comment here to explain why it's ok to just return 0 > here...
Will use
"/* See gmap_make_secure(): large folios cannot be protected */"
> >> + >> + folio_get(folio); >> rc = uv_destroy_page(paddr); >> if (!rc) >> - clear_bit(PG_arch_1, &page->flags); >> - put_page(page); >> + clear_bit(PG_arch_1, &folio->flags); >> + folio_put(folio); >> return rc; >> } >> >> @@ -169,14 +172,17 @@ int uv_convert_from_secure(unsigned long paddr) >> */ >> int uv_convert_owned_from_secure(unsigned long paddr) >> { >> - struct page *page = phys_to_page(paddr); >> + struct folio *folio = phys_to_folio(paddr); >> int rc; >> >> - get_page(page); >> + if (unlikely(folio_test_large(folio))) >> + return 0; > > ... and here
here as well and ...
> >> + >> + folio_get(folio); >> rc = uv_convert_from_secure(paddr); >> if (!rc) >> - clear_bit(PG_arch_1, &page->flags); >> - put_page(page); >> + clear_bit(PG_arch_1, &folio->flags); >> + folio_put(folio); >> return rc; >> } >> >> @@ -457,33 +463,34 @@ EXPORT_SYMBOL_GPL(gmap_destroy_page); >> */ >> int arch_make_page_accessible(struct page *page) >> { >> + struct folio *folio = page_folio(page); >> int rc = 0; >> >> - /* Hugepage cannot be protected, so nothing to do */ >> - if (PageHuge(page)) >> + /* Large folios cannot be protected, so nothing to do */
^ here as well.
>> + if (unlikely(folio_test_large(folio))) >> return 0; >>
-- Cheers,
David / dhildenb
| |