On Sun, Aug 20, 2023 at 9:30 AM Richard Henderson <
richard.hender...@linaro.org> wrote:
> On 8/19/23 02:48, Karim Taha wrote:
> > From: Stacey Son <s...@freebsd.org>
> >
> > Signed-off-by: Stacey Son <s...@freebsd.org>
> > Signed-off-by: Karim Taha <kariem.taha...@gmail.com>
> > ---
> > bsd-user/bsd-mem.h | 72 +++++++++++++++++++++++++++++++++++
> > bsd-user/freebsd/os-syscall.c | 8 ++++
> > 2 files changed, 80 insertions(+)
> >
> > diff --git a/bsd-user/bsd-mem.h b/bsd-user/bsd-mem.h
> > index 221ad76d8c..f737b94885 100644
> > --- a/bsd-user/bsd-mem.h
> > +++ b/bsd-user/bsd-mem.h
> > @@ -335,4 +335,76 @@ static inline abi_long do_bsd_shmctl(abi_long
> shmid, abi_long cmd,
> > return ret;
> > }
> >
> > +/* shmat(2) */
> > +static inline abi_long do_bsd_shmat(int shmid, abi_ulong shmaddr, int
> shmflg)
> > +{
> > + abi_ulong raddr;
> > + abi_long ret;
> > + void *host_raddr;
> > + struct shmid_ds shm_info;
> > + int i;
> > +
> > + /* Find out the length of the shared memory segment. */
> > + ret = get_errno(shmctl(shmid, IPC_STAT, &shm_info));
> > + if (is_error(ret)) {
> > + /* Can't get the length */
> > + return ret;
> > + }
> > +
> > + mmap_lock();
> > +
> > + if (shmaddr) {
> > + host_raddr = shmat(shmid, (void *)g2h_untagged(shmaddr),
> shmflg);
>
> Missing
>
> if (!guest_range_valid_untagged(shmaddr, shm_info.shm_segsz)) {
> return -TARGET_EINVAL;
> }
>
> > + } else {
> > + abi_ulong mmap_start;
> > +
> > + mmap_start = mmap_find_vma(0, shm_info.shm_segsz);
> > +
> > + if (mmap_start == -1) {
> > + errno = ENOMEM;
> > + host_raddr = (void *)-1;
> > + } else {
> > + host_raddr = shmat(shmid, g2h_untagged(mmap_start),
> > + shmflg); /* | SHM_REMAP XXX WHY? */
>
> With reserved_va, the entire guest address space is mapped PROT_NONE so
> that it is
> reserved, so that the kernel does not use it for something else. You need
> the SHM_REMAP
> to replace the reservation mapping.
>
> > +/* shmdt(2) */
> > +static inline abi_long do_bsd_shmdt(abi_ulong shmaddr)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < N_BSD_SHM_REGIONS; ++i) {
> > + if (bsd_shm_regions[i].start == shmaddr) {
> > + bsd_shm_regions[i].start = 0;
> > + page_set_flags(shmaddr,
> > + shmaddr + bsd_shm_regions[i].size, 0);
> > + break;
> > + }
> > + }
> > +
> > + return get_errno(shmdt(g2h_untagged(shmaddr)));
> > +}
>
> Hmm, bug with linux-user as well, because here we should re-establish the
> reserved_va
> reservation.
>
... of the shared memory region we just detached? Right?
> Also, we should not be using a fixed sized array. Nothing good happens
> when the array
> fills up.
>
File this as https://github.com/qemu-bsd-user/qemu-bsd-user/issues/47 so we
don't forget.
It's good enough for the moment since the programs we've seen have a very
limited number
of segments... but longer term, it should be dynamic.
Warner
