[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v7 7/8] xen/riscv: page table handling
On Tue, 2024-09-24 at 15:31 +0200, Jan Beulich wrote: > On 24.09.2024 13:30, oleksii.kurochko@xxxxxxxxx wrote: > > On Tue, 2024-09-24 at 12:49 +0200, Jan Beulich wrote: > > > On 13.09.2024 17:57, Oleksii Kurochko wrote: > > > > +static int pt_next_level(bool alloc_tbl, pte_t **table, > > > > unsigned > > > > int offset) > > > > +{ > > > > + pte_t *entry; > > > > + mfn_t mfn; > > > > + > > > > + entry = *table + offset; > > > > + > > > > + if ( !pte_is_valid(*entry) ) > > > > + { > > > > + if ( !alloc_tbl ) > > > > + return XEN_TABLE_MAP_FAILED; > > > > + > > > > + if ( create_table(entry) ) > > > > + return XEN_TABLE_MAP_FAILED; > > > > > > You're still losing the -ENOMEM here. > > Agree, I will save the return value of create_table and return it. > > That won't work very well, will it? I think it will work, just will be needed another one check in pt_update_entry() where pt_next_level() is called: if ( (rc == XEN_TABLE_MAP_FAILED) || (rc == -ENOMEM) ) ... > Imo you need a new XEN_TABLE_MAP_NOMEM. > (And then XEN_TABLE_MAP_FAILED may want renaming to e.g. > XEN_TABLE_MAP_NONE). I am still curious if we really need this separation. If to in this way then it should be updated the check in pt_update_entry(): --- a/xen/arch/riscv/pt.c +++ b/xen/arch/riscv/pt.c @@ -165,10 +165,10 @@ static int pt_next_level(bool alloc_tbl, pte_t **table, unsigned int offset) if ( !pte_is_valid(*entry) ) { if ( !alloc_tbl ) - return XEN_TABLE_MAP_FAILED; + return XEN_TABLE_MAP_NONE; if ( create_table(entry) ) - return XEN_TABLE_MAP_FAILED; + return XEN_TABLE_MAP_NOMEM; } if ( pte_is_mapping(*entry) ) @@ -209,7 +209,7 @@ static int pt_update_entry(mfn_t root, unsigned long virt, for ( ; level > target; level-- ) { rc = pt_next_level(alloc_tbl, &table, offsets[level]); - if ( rc == XEN_TABLE_MAP_FAILED ) + if ( (rc == XEN_TABLE_MAP_NONE) && (rc == XEN_TABLE_MAP_NOMEM) ) { rc = 0; But the handling of XEN_TABLE_MAP_NONE and XEN_TABLE_MAP_NOMEM seems to me should be left the same as this one part of the code actually catching the case when create_table() returns -ENOMEM: pt_next_level() { ... if ( flags & (PTE_VALID | PTE_POPULATE) ) { dprintk(XENLOG_ERR, "%s: Unable to map level %u\n", __func__, level); rc = -ENOMEM; } ... ~ Oleksii
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |