- User Since
- Feb 20 2021, 4:27 AM (16 w, 4 d)
Sat, Jun 12
...and testing failed, ugh. I stick to my point the event channel numbering in xen_intr.c seems a bit mixed. I'm not entirely sure of the best course for this.
Initial verification passed, final verification is in-progress (previous build failed, but the failure suggested a problem elsewhere and this was fine).
Fri, Jun 11
Add more to comment before xen_intr_isrc_lock. Give explicit condition for unlocking.
Adding not to commit message about not actually being required.
I'm actually tempted to suggest adding:
if (RMAN_IS_DEFAULT_RANGE(start, end)) panic("resource manager initialized with illegal range");
to rman_manage_region(). Too many architectures initialize their address and interrupt rmans to that range, which means they aren't performing a useful function.
One note, seems the issue I was running into was elsewhere. The value of 0 works fine with that issue addressed.
This needs checking, I'm mostly testing on ARM. I think what this does shouldn't recreate the breakage ca7af67ac958d3749ebfc4771cecf0cd7525e6a0 was trying to fix, but I'm not 100% certain.
True enough, I'll add that to the commit message.
Sun, Jun 6
Add a safety check. Protect against left-behind entries in xen_intr_port_to_isrc (shouldn't happen, but just in case).
Fri, Jun 4
Careful review and looks like atomic_store_rel_int() is needed inside isrc_release_counters(). Otherwise stores could move after and that would break. The accesses in isrc_setup_counters() all depend upon "index" which is synchronized.
Any comments on what is currently present?
@mhorne, you got it exactly right. Issue is ofwbus is keeping everything to itself, which is fine if *everything* goes through ofwbus, but a major problem for anything independent of ofwbus (such as Xen PV).
Theory is to make intr_isrc_deregister() work in the general case. Without this (or a fixed version) isrc_release_counters() is of course a wrapper around panic(). What needs checking is I'm unsure of how the atomic_*() functions are supposed to work, I think I got their use right, but please check. Actual hardware removable interrupt sources aren't common, but software removable ones are quite common (VMs).
Thu, Jun 3
Wed, Jun 2
Tue, Jun 1
Appears xen_intr_alloc_and_bind_ipi() was originally pulled to make getting things working easier. Appears it does in fact build on other architectures, but it isn't needed elsewhere. One can argue for or against pulling it on other architectures.
Actually, one question for the FreeBSD/Xen maintainer: Is the order of the xen_intr_port_to_isrc[cur->xi_port] = cur; and evtchn_unmask_port(cur->xi_port); lines important?
I'm now unsure of this. Originally this had been disabling x86-only functionality on other architectures, but the D30598 and D30599 make the current form of these functions build on other architectures. As such there is no longer an urgent need to disable them on other architectures, but they simply don't accomplish anything on non-x86.
This is the main goal of D30598/D30599. The variables xen_intr_auto_vector_count and first_evtchn_irq are x86-specific. The call intr_lookup_source() is also x86-specific which is being utilized to get access to interrupt_sources and use that as a table of allocated event channel interrupts. These two make it so xen_intr_port_to_isrc is used for this purpose instead.
Mon, May 31
The update had been aimed at keeping what was in Phabricator up to date. Unfortunately I still don't think this is quite ready and should still be in the "changed planned" state.
Addressing @royger's comment. The removal of the lapic_eoi() call had been in a distinct commit, but it does make sense to move here.
Sun, May 30
This is a very early tentative proposal. The issue is without this resource allocations which get to the nexus may be given ranges overlapping with what the OpenFirmware code has. Two drivers trying to use the same physical address range will be Bad(tm).
Based on how rm_end = ~0 breaks the resource manager code, using that as the maximum is wrong. I'm unsure of what upper limit should be used, but 2^16 seems reasonable as the maximum interrupt number for the nexus. Perhaps 2^20 to leave a bit more headroom between the maximum allowed by a GICv3 and what some perverse system designer might actually make use of?
Now to get this out of "Changes Planned" since changes elsewhere reduce the concerns I had for a bit with this (accept my own change? @imp had already marked acceptable).
Sat, May 29
Needed to update the commit message too. That also shrinks due to removals.
Mon, May 24
Update due to PVHv1 removal. Quite the chunk of xen_intr_x86.h disappeared
due to the removal of PVHv1. I now wonder if the declaration of
xen_intr_alloc_and_bind_ipi() should be moved to x86/xen-os.h, this might
make other pieces cleaner.
Sun, May 23
Update due to removal of PVHv1, much of the commit is gone, but some
valuable portions remain. xen_handle_shared_info() is still there, the
architecture-independent variables are still here.
Thu, May 20
(one remaining issue is whether I've added the functions in the right place, I'm unsure if perhaps they should be more towards top/bottom of cpu.h and machdep.c)
Comparing with line 52 of sys/x86/acpica/srat.c, it appears acpi_pxm_init() should be getting the maximum valid address as the second argument. This means D26144 was off-by-one. Since the need for the aarch64 nexus is the same value, have the function providing that be the global one. Since I foresee someone wanting the number of address bits or the total count of addresses, leave get_physaddr_bits() and get_physaddr_count() behind with the potential to be made global.
Getting things closer to a ready version. Now a common function, the
two (?) off-by-ones are fixed.
May 17 2021
Yes, in nexus_attach() it would need to be nexus_get_parange() - 1. This happens when you're not really sure what should be done.
May 16 2021
This part is simply the observation, why the heck will bus_alloc_resource(..., SYS_RES_MEMORY, 0, ~0, $large_size,) happily return address ranges (and sizes) which cannot possibly be valid (architecture doesn't allow for processors which have 64 address lines). This simply limits the potential ranges to potentially valid physical addresses.
This is being proposed due to problems being run into when trying to get Xen operational. This is very much preliminary.
Looking at sys/arm64/arm64/nexus.c I see rather major issues and I'm inclined to think the merits of the approach used for x86 is right whereas what is present for aarch64 is wrong.
There is actually a trade-off between two issues here. Is it valuable for rman_reserve_resource_bound() to handle the case of being given a count of 0? Is it valuable for rman_reserve_resource_bound() to handle the situation of count == 0? Both are a bit funky and a bit valuable to handle.
May 15 2021
There are questions of approach for this. While this has demonstrated viability, there are sufficient concerns for this not to be ready yet. The alternative is D29304 which has a distinct issue.
The change planned this isn't ready for commit until it passes tests. I'm pretty sure the problem is elsewhere, but until that is resolved this isn't ready.
This causes a whole bunch of #if defined(__amd64__) || defined(__i386__)/#endif pairs to disappear from unsubmitted commits. For my attempts at getting FreeBSD/ARM operational on Xen/ARM this is a noticeable improvement.
This approach to the problem is still potentially viable and still qualifies as an alternative to D29818. Problem is there is some sort of bug causing this not to work and this needs diagnosis before being committed. I suspect D29694 might have fixed the bug, but this requires a build and test to check current status.
Tested in a build and passed. Can be adjusted on commit, but I suspect this really should be titled "etc/ttys: merge ttys file down to single file". Grabbed the repository file location, instead of the output location when initially committing.
May 13 2021
Follow on for D29873. Clearly the ttys files have been slowly converging, so perhaps it is finally time to go ahead and merge them into a single file.
One item of concern, is the placement of xc0 in the right spot in the list; should it be lower? Very much is not a serial port.
Adding an entry for /etc/ttys for amd64. PVH mode on x86 also has the simulated console.
Placing this back in the queue. I dislike proposing this without D30236 in place, since otherwise this looks like a bizarre commit.
For the purposes of ARM, the API might be better to have a xenpv_set_addr_mgr(struct rman *) function. This allows for the allocated range to come from either device-tree or ACPI table. There are two outstanding issues.
Changes to other commits overlapped this enough to make it no longer worthwhile as a separate commit.
This isn't quite what I had expected, I was expecting more of cfa0b7b82fbdda56d7160569def5c6133eb045aa to be reverted. That though appears to have been done in a separate commit which isn't here.
May 12 2021
May 8 2021
Updating D29875 to what I currently have, though I do not expect this to be final either. Mostly this is updating others on what I have. General idea is the call to xen_domain() at the top of xencons_cnprobe() could be replaced with a call to xen_early_probe(). On x86 xen_early_probe() would simply be a #define to xen_domain(), whereas on other architectures the console probe is when Xen's presence or absence needs to be known.
Now with a much larger commit message. Notably this approach distinctly shrinks isrc_alloc_irq(). I like preserving the features which enhance performance. I'll admit some of the simplification is due to D29327 making maxirqs completely unnecessary (its purpose was to provide an unsigned version of intr_nirq).
Updating with the latest from my current successful build. The PVHv1 portion may end up removed due to discontinuing support for PVHv1, but the handling of the shared_info page should be here. Plus hvm_start_flags and xen_domain_type become common.
May 4 2021
Switching to SI_ORDER_SECOND from SI_ORDER_FIRST. With hypervisors, SI_ORDER_FIRST should be reserved for probing, whereas this is merely very early setup.
May 3 2021
Marking D29598 as changed planned. This needs to be tested for whether this is now unneeded.
Think about this, should D29873 add the Xen virtual console device to the sbin/init/ttys.* files for all architectures? This would keep these files consistent and at least one other architecture is looking at potential Xen support. My only question would be the treatment of x86. As of FreeBSD 12, the Xen code was using the emulated serial port as console instead of the ring. Should the ring device be added for x86 even though it currently isn't used?
As a revision, I think this is mostly ready. Problem is due to taking the role of feeding interrupts to the rest of the Xen interface, it really needs the interrupt work done first.
Revision Actions => Plan Changes
May 2 2021
Several of the comments have been addressed, yet the issue pointed to during upload now looms large. This looks like it should come after the interrupt code situation is resolved.
Still not in final shape.
Then I realize this really should be part of the work with the Xen interrupt work (which could still end up as a rewrite instead of being based off sys/x86/xen/xen_intr.c).
May 1 2021
Update creating xen_handle_shared_info(). Idea is to share between x86 and
other architectures. This uses an approach similar to the one proposed for
ARM, but pieces have been inspired by the x86 implementation.
Apr 28 2021
Updating to what I currently have. I do not think this is final.
Apr 27 2021
Hmm, updated this locally and hadn't gotten it uploaded. Simply placing a #error in x86/xen/xen-os.h to prevent direct inclusion.
This is on my queue for investigation (alas, this is behind the queue for being submitted).
This is an early phase of getting interrupts operational on ARM. Simply figuring out which functions are actually used in common files (or in case of xen_intr_handle_upcall() needs to be invoked). @julien_xen.org originally implemented it as moving sys/x86/xen/xen_intr.c to sys/xen/xen_intr.c, but I'm wondering whether it is better to start from scratch. The #ifdef in sys/x86/xen/xen_intr.c is a precursor to @julien_xen.org's approach. If it was reimplemented from scratch that would end up useless, but for now I'm keeping that option open.
The idea behind this is non-x86 really needs a function which can be passed to bus_setup_intr(). This results in two differences which I don't believe need any modifications what-so-ever to x86. First, instead of taking struct trap_frame * it takes void *.
Second, instead of being void, it returns int.
I think what is in D29874 is going to merge into D29875. I've got comments resolved in my tree. Could be a bit before I get things updated here though. This has turned back into a pile of misc patches and needs to get turned back into a patch.
Apr 26 2021
Thing is my goal is to get Xen/ARM working. If removing PVHv1 support made that noticeably easier, I would be for that. Yet here it makes minimal difference to simply leave it alone and removing it would require me to check I hadn't broken anything.