Re: git: a8b4678fc925 - main - emulators/virtualbox-ose-70: Fix 15-CURRENT kmod build

From: Cy Schubert <Cy.Schubert_at_cschubert.com>
Date: Mon, 28 Apr 2025 23:29:39 UTC
On Tue, 29 Apr 2025 00:59:36 +0300
Vladimir Druzenko <vvd@freebsd.org> wrote:

> Hello!
> 
> Thanks for patch, but!
> 1. What is the purpose of such changes:
> > -@@ -155,8 +157,6 @@ DECLHIDDEN(int) rtR0MemObjNativeFree(RTR0MEMOBJ pMem)
> > +@@ -155,8 +157,6 @@  
> ?

Patch regeneration. diff -u produces a new patch.

> 
> 2. AFAIK, I'm the only one in vbox@ (maintainer of all VirtualBox ports) 
> and I'm active. Why did you committed this patch without at least notify me?

Build fixes are implicitly approved by portmgr. I should have put "approved by: portmgr (break fix)". 

> It isn't "port breakage" - 15 isn't releases yet and we can prepare 

Actually it is port breakage.

--- memuserkernel-r0drv-freebsd.o ---
--- memobj-r0drv-freebsd.o ---
/wrkdirs/usr/ports/emulators/virtualbox-ose-kmod-70/work/VirtualBox-7.0.24/out/freebsd.amd64/release/bin/src/vboxdrv/r0drv/freebsd/memobj-r0drv-freebsd.c:474:53: error: member reference type 'int' is not a pointer
  474 |                 pMemFreeBSD->Core.u.Phys.PhysBase = VM_PAGE_TO_PHYS(vm_page_find_least(pMemFreeBSD->pObject, 0));
      |                                                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/src/sys/vm/vm_page.h:504:42: note: expanded from macro 'VM_PAGE_TO_PHYS'
  504 | #define VM_PAGE_TO_PHYS(entry)  ((entry)->phys_addr)
      |                                  ~~~~~~~  ^
1 error generated.
*** [memobj-r0drv-freebsd.o] Error code 1


> patch several days as was in:
> https://bugs.freebsd.org/286206
> https://bugs.freebsd.org/286193
> https://bugs.freebsd.org/286204
> If you want, you can join vbox@.

Sure.

> Otherwise, it is unclear what these groups and maintainers are for, if 
> everyone can commit whatever they want at their own discretion.

As I understand, break fixes are implicitly approved by portmgr.

> 
> 3. Have you checked other versions of VirtualBox: 
> emulators/virtualbox-ose-kmod and emulators/virtualbox-ose-kmod-legacy?
> Do they need the same patch?

They all need the patch since vm_page_find_least() was removed from 15.
This affected not only the vbox ports but the graphics/drm* ports as
well.

> 
> 
> Please either fix the patch taking into account the comments above, and 
> also prepare patches for other versions of VirtualBox, or revert the 
> commit and create a PR with the proposed patch, and I will fix it myself 
> and port it to other versions of VirtualBox (if necessary).
> 
> If you don't do any of the above, then I, as the maintainer, will do 
> everything myself on May 1st.

I can apply the same fixes to all the vbox ports if you want or if you
prefer, that's fine by me as well.

Sorry for stepping on your toes here. It wasn't my intention to take
over, just to fix a broken port. I'm perfectly fine only opening PRs for
the vbox ports if you prefer.

> 
> 
> 28.04.2025 23:32, Cy Schubert пишет:
> > The branch main has been updated by cy:
> >
> > URL: https://cgit.FreeBSD.org/ports/commit/?id=a8b4678fc9255e3c2c95598c95bf5972941998fd
> >
> > commit a8b4678fc9255e3c2c95598c95bf5972941998fd
> > Author:     Cy Schubert <cy@FreeBSD.org>
> > AuthorDate: 2025-04-28 17:19:13 +0000
> > Commit:     Cy Schubert <cy@FreeBSD.org>
> > CommitDate: 2025-04-28 20:29:16 +0000
> >
> >      emulators/virtualbox-ose-70: Fix 15-CURRENT kmod build
> >      
> >      One remaining instance of vm_page_find_least(), which was removed in
> >      15-CURRENT, remained. Replace it with a call to vm_radix_lookup_ge()
> >      instead.
> > ---
> >   ...ox_Runtime_r0drv_freebsd_memobj-r0drv-freebsd.c | 80 +++++++++++++---------
> >   1 file changed, 46 insertions(+), 34 deletions(-)
> >
> > diff --git a/emulators/virtualbox-ose-70/files/patch-src_VBox_Runtime_r0drv_freebsd_memobj-r0drv-freebsd.c b/emulators/virtualbox-ose-70/files/patch-src_VBox_Runtime_r0drv_freebsd_memobj-r0drv-freebsd.c
> > index 7b8767a8aa58..205c897c818d 100644
> > --- a/emulators/virtualbox-ose-70/files/patch-src_VBox_Runtime_r0drv_freebsd_memobj-r0drv-freebsd.c
> > +++ b/emulators/virtualbox-ose-70/files/patch-src_VBox_Runtime_r0drv_freebsd_memobj-r0drv-freebsd.c
> > @@ -1,6 +1,6 @@
> > ---- src/VBox/Runtime/r0drv/freebsd/memobj-r0drv-freebsd.c.orig	2025-01-21 14:07:00 UTC
> > -+++ src/VBox/Runtime/r0drv/freebsd/memobj-r0drv-freebsd.c
> > -@@ -139,8 +139,10 @@ DECLHIDDEN(int) rtR0MemObjNativeFree(RTR0MEMOBJ pMem)
> > +--- src/VBox/Runtime/r0drv/freebsd/memobj-r0drv-freebsd.c.orig	2025-04-11 05:12:39.000000000 -0700
> > ++++ src/VBox/Runtime/r0drv/freebsd/memobj-r0drv-freebsd.c	2025-04-28 10:14:45.981609000 -0700
> > +@@ -139,8 +139,10 @@
> >    
> >    DECLHIDDEN(int) rtR0MemObjNativeFree(RTR0MEMOBJ pMem)
> >    {
> > @@ -11,7 +11,7 @@
> >    
> >        switch (pMemFreeBSD->Core.enmType)
> >        {
> > -@@ -155,8 +157,6 @@ DECLHIDDEN(int) rtR0MemObjNativeFree(RTR0MEMOBJ pMem)
> > +@@ -155,8 +157,6 @@
> >    
> >            case RTR0MEMOBJTYPE_LOCK:
> >            {
> > @@ -20,7 +20,7 @@
> >                if (pMemFreeBSD->Core.u.Lock.R0Process != NIL_RTR0PROCESS)
> >                    pMap = &((struct proc *)pMemFreeBSD->Core.u.Lock.R0Process)->p_vmspace->vm_map;
> >    
> > -@@ -197,6 +197,7 @@ DECLHIDDEN(int) rtR0MemObjNativeFree(RTR0MEMOBJ pMem)
> > +@@ -197,6 +197,7 @@
> >            case RTR0MEMOBJTYPE_PHYS_NC:
> >            {
> >                VM_OBJECT_WLOCK(pMemFreeBSD->pObject);
> > @@ -28,10 +28,11 @@
> >                vm_page_t pPage = vm_page_find_least(pMemFreeBSD->pObject, 0);
> >    #if __FreeBSD_version < 1000000
> >                vm_page_lock_queues();
> > -@@ -210,6 +211,14 @@ DECLHIDDEN(int) rtR0MemObjNativeFree(RTR0MEMOBJ pMem)
> > +@@ -209,6 +210,14 @@
> > +             }
> >    #if __FreeBSD_version < 1000000
> >                vm_page_unlock_queues();
> > - #endif
> > ++#endif
> >   +#else /* __FreeBSD_version >= 1500038 */
> >   +            struct pctrie_iter pages;
> >   +            vm_page_t page;
> > @@ -39,11 +40,10 @@
> >   +            pctrie_iter_init(&pages, pMemFreeBSD->pObject);
> >   +            VM_RADIX_FORALL(page, &pages)
> >   +                vm_page_unwire(page, PQ_INACTIVE);
> > -+#endif
> > + #endif
> >                VM_OBJECT_WUNLOCK(pMemFreeBSD->pObject);
> >                vm_object_deallocate(pMemFreeBSD->pObject);
> > -             break;
> > -@@ -220,6 +229,7 @@ DECLHIDDEN(int) rtR0MemObjNativeFree(RTR0MEMOBJ pMem)
> > +@@ -220,6 +229,7 @@
> >                return VERR_INTERNAL_ERROR;
> >        }
> >    
> > @@ -51,7 +51,7 @@
> >        return VINF_SUCCESS;
> >    }
> >    
> > -@@ -329,7 +339,8 @@ static int rtR0MemObjFreeBSDAllocHelper(PRTR0MEMOBJFRE
> > +@@ -329,7 +339,8 @@
> >        size_t      cPages = atop(pMemFreeBSD->Core.cb);
> >        int         rc;
> >    
> > @@ -61,7 +61,7 @@
> >    
> >        /* No additional object reference for auto-deallocation upon unmapping. */
> >    #if __FreeBSD_version >= 1000055
> > -@@ -371,6 +382,7 @@ DECLHIDDEN(int) rtR0MemObjNativeAllocPage(PPRTR0MEMOBJ
> > +@@ -371,6 +382,7 @@
> >    
> >    DECLHIDDEN(int) rtR0MemObjNativeAllocPage(PPRTR0MEMOBJINTERNAL ppMem, size_t cb, bool fExecutable, const char *pszTag)
> >    {
> > @@ -69,7 +69,7 @@
> >        PRTR0MEMOBJFREEBSD pMemFreeBSD = (PRTR0MEMOBJFREEBSD)rtR0MemObjNew(sizeof(*pMemFreeBSD), RTR0MEMOBJTYPE_PAGE,
> >                                                                           NULL, cb, pszTag);
> >        if (pMemFreeBSD)
> > -@@ -380,8 +392,10 @@ DECLHIDDEN(int) rtR0MemObjNativeAllocPage(PPRTR0MEMOBJ
> > +@@ -380,8 +392,10 @@
> >                *ppMem = &pMemFreeBSD->Core;
> >            else
> >                rtR0MemObjDelete(&pMemFreeBSD->Core);
> > @@ -80,7 +80,7 @@
> >        return VERR_NO_MEMORY;
> >    }
> >    
> > -@@ -395,6 +409,7 @@ DECLHIDDEN(int) rtR0MemObjNativeAllocLow(PPRTR0MEMOBJI
> > +@@ -395,6 +409,7 @@
> >    
> >    DECLHIDDEN(int) rtR0MemObjNativeAllocLow(PPRTR0MEMOBJINTERNAL ppMem, size_t cb, bool fExecutable, const char *pszTag)
> >    {
> > @@ -88,7 +88,7 @@
> >        PRTR0MEMOBJFREEBSD pMemFreeBSD = (PRTR0MEMOBJFREEBSD)rtR0MemObjNew(sizeof(*pMemFreeBSD), RTR0MEMOBJTYPE_LOW, NULL, cb, pszTag);
> >        if (pMemFreeBSD)
> >        {
> > -@@ -403,14 +418,17 @@ DECLHIDDEN(int) rtR0MemObjNativeAllocLow(PPRTR0MEMOBJI
> > +@@ -403,14 +418,17 @@
> >                *ppMem = &pMemFreeBSD->Core;
> >            else
> >                rtR0MemObjDelete(&pMemFreeBSD->Core);
> > @@ -106,7 +106,7 @@
> >        PRTR0MEMOBJFREEBSD pMemFreeBSD = (PRTR0MEMOBJFREEBSD)rtR0MemObjNew(sizeof(*pMemFreeBSD), RTR0MEMOBJTYPE_CONT,
> >                                                                           NULL, cb, pszTag);
> >        if (pMemFreeBSD)
> > -@@ -423,8 +441,10 @@ DECLHIDDEN(int) rtR0MemObjNativeAllocCont(PPRTR0MEMOBJ
> > +@@ -423,8 +441,10 @@
> >            }
> >            else
> >                rtR0MemObjDelete(&pMemFreeBSD->Core);
> > @@ -117,7 +117,7 @@
> >        return VERR_NO_MEMORY;
> >    }
> >    
> > -@@ -432,6 +452,7 @@ static int rtR0MemObjFreeBSDAllocPhysPages(PPRTR0MEMOB
> > +@@ -432,6 +452,7 @@
> >    static int rtR0MemObjFreeBSDAllocPhysPages(PPRTR0MEMOBJINTERNAL ppMem, RTR0MEMOBJTYPE enmType, size_t cb,  RTHCPHYS PhysHighest,
> >                                               size_t uAlignment, bool fContiguous, int rcNoMem, const char *pszTag)
> >    {
> > @@ -125,7 +125,7 @@
> >        /* create the object. */
> >        PRTR0MEMOBJFREEBSD pMemFreeBSD = (PRTR0MEMOBJFREEBSD)rtR0MemObjNew(sizeof(*pMemFreeBSD), enmType, NULL, cb, pszTag);
> >        if (pMemFreeBSD)
> > -@@ -439,7 +460,8 @@ static int rtR0MemObjFreeBSDAllocPhysPages(PPRTR0MEMOB
> > +@@ -439,7 +460,8 @@
> >            vm_paddr_t const VmPhysAddrHigh = PhysHighest != NIL_RTHCPHYS ? PhysHighest : ~(vm_paddr_t)0;
> >            u_long const     cPages         = atop(cb);
> >    
> > @@ -135,7 +135,19 @@
> >    
> >            int rc = rtR0MemObjFreeBSDPhysAllocHelper(pMemFreeBSD->pObject, cPages, VmPhysAddrHigh,
> >                                                      uAlignment, fContiguous, true, rcNoMem);
> > -@@ -462,8 +484,10 @@ static int rtR0MemObjFreeBSDAllocPhysPages(PPRTR0MEMOB
> > +@@ -449,7 +471,11 @@
> > +             {
> > +                 Assert(enmType == RTR0MEMOBJTYPE_PHYS);
> > +                 VM_OBJECT_WLOCK(pMemFreeBSD->pObject);
> > ++#if __FreeBSD_version < 1500038
> > +                 pMemFreeBSD->Core.u.Phys.PhysBase = VM_PAGE_TO_PHYS(vm_page_find_least(pMemFreeBSD->pObject, 0));
> > ++#else
> > ++                pMemFreeBSD->Core.u.Phys.PhysBase = VM_PAGE_TO_PHYS(vm_radix_lookup_ge(&(pMemFreeBSD->pObject->rtree), 0));
> > ++#endif
> > +                 VM_OBJECT_WUNLOCK(pMemFreeBSD->pObject);
> > +                 pMemFreeBSD->Core.u.Phys.fAllocated = true;
> > +             }
> > +@@ -462,8 +488,10 @@
> >                vm_object_deallocate(pMemFreeBSD->pObject);
> >                rtR0MemObjDelete(&pMemFreeBSD->Core);
> >            }
> > @@ -146,7 +158,7 @@
> >        return VERR_NO_MEMORY;
> >    }
> >    
> > -@@ -486,6 +510,7 @@ DECLHIDDEN(int) rtR0MemObjNativeEnterPhys(PPRTR0MEMOBJ
> > +@@ -486,6 +514,7 @@
> >                                              const char *pszTag)
> >    {
> >        AssertReturn(uCachePolicy == RTMEM_CACHE_POLICY_DONT_CARE, VERR_NOT_SUPPORTED);
> > @@ -154,7 +166,7 @@
> >    
> >        /* create the object. */
> >        PRTR0MEMOBJFREEBSD pMemFreeBSD = (PRTR0MEMOBJFREEBSD)rtR0MemObjNew(sizeof(*pMemFreeBSD), RTR0MEMOBJTYPE_PHYS,
> > -@@ -497,8 +522,10 @@ DECLHIDDEN(int) rtR0MemObjNativeEnterPhys(PPRTR0MEMOBJ
> > +@@ -497,8 +526,10 @@
> >            pMemFreeBSD->Core.u.Phys.PhysBase = Phys;
> >            pMemFreeBSD->Core.u.Phys.uCachePolicy = uCachePolicy;
> >            *ppMem = &pMemFreeBSD->Core;
> > @@ -165,7 +177,7 @@
> >        return VERR_NO_MEMORY;
> >    }
> >    
> > -@@ -510,6 +537,7 @@ static int rtR0MemObjNativeLockInMap(PPRTR0MEMOBJINTER
> > +@@ -510,6 +541,7 @@
> >                                         vm_offset_t AddrStart, size_t cb, uint32_t fAccess,
> >                                         RTR0PROCESS R0Process, int fFlags, const char *pszTag)
> >    {
> > @@ -173,7 +185,7 @@
> >        int rc;
> >        NOREF(fAccess);
> >    
> > -@@ -519,21 +547,28 @@ static int rtR0MemObjNativeLockInMap(PPRTR0MEMOBJINTER
> > +@@ -519,21 +551,28 @@
> >        if (!pMemFreeBSD)
> >            return VERR_NO_MEMORY;
> >    
> > @@ -210,7 +222,7 @@
> >        return VERR_NO_MEMORY;/** @todo fix mach -> vbox error conversion for freebsd. */
> >    }
> >    
> > -@@ -573,6 +608,7 @@ static int rtR0MemObjNativeReserveInMap(PPRTR0MEMOBJIN
> > +@@ -573,6 +612,7 @@
> >    static int rtR0MemObjNativeReserveInMap(PPRTR0MEMOBJINTERNAL ppMem, void *pvFixed, size_t cb, size_t uAlignment,
> >                                            RTR0PROCESS R0Process, vm_map_t pMap, const char *pszTag)
> >    {
> > @@ -218,7 +230,7 @@
> >        int rc;
> >    
> >        /*
> > -@@ -631,11 +667,13 @@ static int rtR0MemObjNativeReserveInMap(PPRTR0MEMOBJIN
> > +@@ -631,11 +671,13 @@
> >            pMemFreeBSD->Core.pv = (void *)MapAddress;
> >            pMemFreeBSD->Core.u.ResVirt.R0Process = R0Process;
> >            *ppMem = &pMemFreeBSD->Core;
> > @@ -232,7 +244,7 @@
> >        return rc;
> >    
> >    }
> > -@@ -659,6 +697,8 @@ DECLHIDDEN(int) rtR0MemObjNativeMapKernel(PPRTR0MEMOBJ
> > +@@ -659,6 +701,8 @@
> >    DECLHIDDEN(int) rtR0MemObjNativeMapKernel(PPRTR0MEMOBJINTERNAL ppMem, RTR0MEMOBJ pMemToMap, void *pvFixed, size_t uAlignment,
> >                                              unsigned fProt, size_t offSub, size_t cbSub, const char *pszTag)
> >    {
> > @@ -241,7 +253,7 @@
> >    //  AssertMsgReturn(!offSub && !cbSub, ("%#x %#x\n", offSub, cbSub), VERR_NOT_SUPPORTED);
> >        AssertMsgReturn(pvFixed == (void *)-1, ("%p\n", pvFixed), VERR_NOT_SUPPORTED);
> >    
> > -@@ -713,6 +753,7 @@ DECLHIDDEN(int) rtR0MemObjNativeMapKernel(PPRTR0MEMOBJ
> > +@@ -713,6 +757,7 @@
> >                Assert((vm_offset_t)pMemFreeBSD->Core.pv == Addr);
> >                pMemFreeBSD->Core.u.Mapping.R0Process = NIL_RTR0PROCESS;
> >                *ppMem = &pMemFreeBSD->Core;
> > @@ -249,7 +261,7 @@
> >                return VINF_SUCCESS;
> >            }
> >            rc = vm_map_remove(kernel_map, Addr, Addr + cbSub);
> > -@@ -721,6 +762,7 @@ DECLHIDDEN(int) rtR0MemObjNativeMapKernel(PPRTR0MEMOBJ
> > +@@ -721,6 +766,7 @@
> >        else
> >            vm_object_deallocate(pMemToMapFreeBSD->pObject);
> >    
> > @@ -257,7 +269,7 @@
> >        return VERR_NO_MEMORY;
> >    }
> >    
> > -@@ -728,6 +770,8 @@ DECLHIDDEN(int) rtR0MemObjNativeMapUser(PPRTR0MEMOBJIN
> > +@@ -728,6 +774,8 @@
> >    DECLHIDDEN(int) rtR0MemObjNativeMapUser(PPRTR0MEMOBJINTERNAL ppMem, RTR0MEMOBJ pMemToMap, RTR3PTR R3PtrFixed, size_t uAlignment,
> >                                            unsigned fProt, RTR0PROCESS R0Process, size_t offSub, size_t cbSub, const char *pszTag)
> >    {
> > @@ -266,7 +278,7 @@
> >        /*
> >         * Check for unsupported stuff.
> >         */
> > -@@ -785,44 +829,50 @@ DECLHIDDEN(int) rtR0MemObjNativeMapUser(PPRTR0MEMOBJIN
> > +@@ -785,44 +833,50 @@
> >    
> >        if (rc == KERN_SUCCESS)
> >        {
> > @@ -321,7 +333,7 @@
> >    
> >        if ((fProt & RTMEM_PROT_NONE) == RTMEM_PROT_NONE)
> >            ProtectionFlags = VM_PROT_NONE;
> > -@@ -833,7 +883,12 @@ DECLHIDDEN(int) rtR0MemObjNativeProtect(PRTR0MEMOBJINT
> > +@@ -833,7 +887,12 @@
> >        if ((fProt & RTMEM_PROT_EXEC) == RTMEM_PROT_EXEC)
> >            ProtectionFlags |= VM_PROT_EXECUTE;
> >    
> > @@ -334,7 +346,7 @@
> >        if (krc == KERN_SUCCESS)
> >            return VINF_SUCCESS;
> >    
> > -@@ -858,11 +913,19 @@ DECLHIDDEN(RTHCPHYS) rtR0MemObjNativeGetPagePhysAddr(P
> > +@@ -858,11 +917,19 @@
> >    
> >                vm_offset_t pb = (vm_offset_t)pMemFreeBSD->Core.pv + ptoa(iPage);
> >    
> > @@ -358,7 +370,7 @@
> >            }
> >    
> >            case RTR0MEMOBJTYPE_MAPPING:
> > -@@ -871,11 +934,15 @@ DECLHIDDEN(RTHCPHYS) rtR0MemObjNativeGetPagePhysAddr(P
> > +@@ -871,11 +938,15 @@
> >    
> >                if (pMemFreeBSD->Core.u.Mapping.R0Process != NIL_RTR0PROCESS)
> >                {
> > @@ -375,7 +387,7 @@
> >                }
> >                return vtophys(pb);
> >            }
> > -@@ -886,9 +953,11 @@ DECLHIDDEN(RTHCPHYS) rtR0MemObjNativeGetPagePhysAddr(P
> > +@@ -886,9 +957,11 @@
> >            {
> >                RTHCPHYS addr;
> >      
> 



-- 
Cheers,
Cy Schubert <Cy.Schubert@cschubert.com>
FreeBSD UNIX:  <cy@FreeBSD.org>   Web:  https://FreeBSD.org
NTP:           <cy@nwtime.org>    Web:  https://nwtime.org

			e^(i*pi)+1=0