On 05.07.2025 16:27, Koichiro Den wrote: > --- a/xen/arch/arm/mm.c > +++ b/xen/arch/arm/mm.c > @@ -180,7 +180,21 @@ int xenmem_add_to_physmap_one( > case XENMAPSPACE_dev_mmio: > rc = map_dev_mmio_page(d, gfn, _mfn(idx)); > return rc; > + case XENMAPSPACE_pv_time: > +#ifdef CONFIG_ARM_64
These two lines want to change places, I think. > + ASSERT(IS_POWER_OF_TWO(sizeof(struct pv_time_region))); BUILD_BUG_ON() please, so that an issue here can be spotted at build time rather than only at runtime. > + if ( idx >= DIV_ROUND_UP(d->max_vcpus * sizeof(struct > pv_time_region), > + PAGE_SIZE) ) > + return -EINVAL; > + > + if ( idx == 0 ) > + d->arch.pv_time_regions_gfn = gfn; This looks fragile, as it'll break once d->max_vcpus can grow large enough to permit a non-zero idx by way of the earlier if() falling through. Imo you'll need at least one further BUILD_BUG_ON() here. > > + mfn = virt_to_mfn(d->arch.pv_time_regions[idx]); > + t = p2m_ram_ro; Is this the correct type to use here? That is, do you really mean guest write attempts to be silently dropped, rather than being reported to the guest as a fault? Then again I can't see such behavior being implemented on Arm, despite the comment on the enumerator saying so (likely inherited from x86). > + break; > +#endif > default: > return -ENOSYS; > } As to style: Please, rather than absorbing the blank line that was there, make sure non-fall-through case blocks are separated from adjacent ones by a blank line. > --- a/xen/include/public/memory.h > +++ b/xen/include/public/memory.h > @@ -217,6 +217,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_machphys_mapping_t); > Stage-2 using the Normal Memory > Inner/Outer Write-Back Cacheable > memory attribute. */ > +#define XENMAPSPACE_pv_time 6 /* PV time shared region (ARM64 only) */ The comment isn't specific enough. As per the struct declaration in patch 4, this interface is solely about stolen time. There's a wider PV interface, which at least x86 Linux also uses, and which has been adopted by KVM as well iirc. Hence this new type wants to clarify what exactly it's used for right now, while leaving open other uses on other architectures. Jan
