Re: API change request. Was => Re: Time accounting on statclock()

[Cherry G.Mathew <cherry%zyx.in@localhost>]

Cherry G.Mathew <cherry%zyx.in@localhost> writes:

> matthew green <mrg%eterna.com.au@localhost> writes:
>
> [...]
>
>>
>> what problem are you actually trying to solve?
>>
>
> Ok, so I think I now have enough info to compose everything into a
> coherent question.
>
> Motivation:
>
> Unifying XEN vector.S entry points with x86/ native ones.
>
> The catch here is that in PV mode, the interrupt entry point is a single
> address, which serves as a demultiplexer for one or more interrupts that
> have to be routed to this domain since the last time the hypervisor
> noticed.
>
> What needs to happen is that the demultiplexing function needs to scan a
> bitmap which contains the set of pending interrupts, and then do the
> regular mask/eoi/service/unmask dance which is time sensitive in ways
> that I haven't got my head around fully yet.
>
> Design:
>
> What I've done therefore is to use the queue/despatch pattern that other
> OSs use (OpenBSD's implementation is the closest to our codebase) so
> that the bitscan queues up handlers to be executed later, and when the
> demultiplexer is done, it can despatch the queued up handlers.
>
> What I quickly realised is that the spl(9) api is effectively a
> scheduler for pending interrupts, and can be effectively abused to do
> the queue/schedule dance.
>
> Implementation:
>
> Here's a snippet:
>
> +/* This is essentially a despatch function for queued interrupts */
> +static void
> +xen_despatch_events(struct cpu_info *ci)
> +{
> +       int i, spl;
> +       u_long psl;
> +
> +       spl = splraise(IPL_HYPERVISOR);
> +       /*
> +        * This bit uses spl(9) magic to brute force calling every
> +        * pending handler at every SPL level down to the interuptee.
> +        */
> +
> +       for (i = IPL_HYPERVISOR;ci->ci_ipending && i >= spl;i--) {
> +               psl = x86_read_psl();
> +
> +               x86_enable_intr();
> +               spllower(i);
> +               x86_write_psl(psl);
> +       }
> +       spllower(spl);
> +}
> +
> +static void
> +xen_queue_event(struct cpu_info *ci, unsigned long port)
> +{
> +
> +       /* Set pending bit for spl code 
> +        * effectively atomic_or_32(&ci->ci_ipending,
> +        * evtsource[port]->ev_imask);
> +        * + some xen glue
> +        */
> +
> +       hypervisor_set_ipending(evtsource[port]->ev_imask,
> +           port >> LONG_SHIFT, port & LONG_MASK);
> +
> +}
> +
>
> Problem:
>
> This works nicely - I tried it on -current and everything just works.
>
> EXCEPT
>
> hardclock(9) and friends inspect the trapframe for various queries. The
> ones I enumerated in our kernel are:
>
> CLKF_USERMODE(frame)
> CLKF_PC(frame)
> CLKF_INTR(frame)
>
> spllower(9) however invents its own stackframe based on the caller's
> stack context. This is problematic for the above abuse because
> spllower(9) is always called from kernel mode, the spllower() code
> returns via regular vector.S interrupt return mechanisms which means
> that we can't just mess with the stack frame here to fake what we want
> for, say a frame we want hardclock() to think came from usermode. (It
> will effectively return to kernel in user mode).
>
> Proposed solution:
>
> change hardclock(9) , statclock(9) and dtrace related hooks that use the
> interrupt frame to have the info they need passed in explicitly.
>
> For eg:
>
> void
> hardclock(struct clockframe *); changes to
>
> changes to:
>
> void
> hardclock(vaddr_t pc, bool usermode, bool intrp);
>
> Hope this makes sense.
>
> Many Thanks,

For now, I did a little arch specific (XEN) hack:
https://mail-index.netbsd.org/source-changes/2018/11/18/msg100702.html~

-- 
~cherry