...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).

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.

Add a safety check. Protect against left-behind entries in xen_intr_port_to_isrc[] (shouldn't happen, but just in case).

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).

Moving the declaration removal to D30605. Though this does really make D30605 and D29913 rather small. Used to be larger until the PVHv1 removal.

Rebalancing between D29913 and D30605; D29913 should really only be the variable move.

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.

Get the ordering right. This really needs D30236 to make sense; I've got it after D30599 in my tree, but there isn't any actual ordering issue with D30599.

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.

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.

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).

Needed to update the commit message too. That also shrinks due to removals.

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.

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.

(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.

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.

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.

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.

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.

In D29873#675696, @imp wrote:

The only reason to maybe add it is that our ttys are the same everywhere and we'd like to go to 1 ttys since the need to have it be radically different has passed away.

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.

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.

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.

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?

Marking D29874 as abandoned as the hunks have been scattered. The initial probe code has moved back to the device-tree code with D29875. The shared_info portions have joined D28982.

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

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).

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.

Updating to what I currently have. I do not think this is final.

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).

D29811 could use some attention. It is marked as parent for several since those break on x86 without D29811 (wrong order of function declaration, several call xen_*_domain()).

I'm thinking what is xen_init() here is going to be merged into D28982. While D30006 would make the interrupt bit cleaner (assuming that actually works for x86).

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.

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.