Add test from PR 294054

Update the commit message with the list of vars allowed in the commit.
Provide the whole list of changeable vars in rtld_get_var(3).

Does the extern declaration for __elfN(fallback_brand) in sys/sys/imgact_elf.h is needed after the patch? Could the __elfN(fallback_brand) var made static in kern_elf.c?

Unrelated note: it seems that pipe fds leak to both pd13 and pd23 processes. This is perhaps not too important but just not accurate. And since we care about 'security' by entering cap mode, ability of the children to affect input of siblings or parent might be considered undesirable.

Note that dangerous_ld_env is never reset back to false.

More uses for rtld_recalc_bind_not().

Handle recalculations second-order effects.

Do call on_update()

I mean something like this (ignore naming), in the generic ioctl handler, not in the driver code;

case SIOCGI2C_OLD:
     i2c.page = i2c.bank = 0;
     ioctl(SIOCG2C, &i2c);
     break;

I.e. drivers only need to implement the new ioctl that knows about bank/page.

In D56022#1282270, @aokblast wrote:

In D56022#1281980, @kib wrote:

No. Perhaps it is simpler to show code than to try to explain it, See D56053.

The example you provided would not work with my patch, it requires increasing the CXA_DTORS_ITERATIONS. Not sure if we want this.

If it breaks the valid C++ code, we should rethink about it. The T value can be somehow greater than CXA_DTORS_ITERATIONS if the user wants.

No. Perhaps it is simpler to show code than to try to explain it, See D56053.

Move fred check into setidt().

In D56005#1281766, @royger wrote:

It might be easier if I just switch Xen to use lapic_ipi_alloc() like mentioned in the other review. That would prevent more Xen stuff leaking into generic headers which is not specially nice.

Do we need to handle the rest of the fenv functions same, they all are extern inline. Then we can remove the requirement to have gcc semantic for inline as a bonus.

In D55915#1281382, @timo.voelker_fh-muenster.de wrote:

In D55915#1281289, @kib wrote:

In D55915#1281101, @timo.voelker_fh-muenster.de wrote:

In D55915#1280245, @kib wrote:

You only write a single value into the specified offset, right?
This should be clearly explained in the man page addition.
Or consider adding a possibility of writing the series of values starting at the given offset, of the specified width for each value (IMO it is fine as is, but needs to be properly documented).

In my use case, I only had to write a single value. Do you know use cases where a series of values needs to be written to a BAR region?

I tried to make it similar to -w (write to config space). This is the man page part about -w:

The -w option writes the value into a configuration space register at byte offset addr of device selector.

Do you find this more clear?

Yes

OK. This is the synopsis:

pciconf -Bw [-b | -h | -x] device bar offset value

and this is the current text:

The -Bw option writes to the BAR specified by its offset in config space bar. The value is written at byte offset offset in the BAR region.

How about this?

The -Bw option writes the value into a BAR region of a PCI device at the given byte offset. It uses the BAR specified by its offset in the config space bar of the PCI device specified by the selector device.

I am on edge of proposing to count the number of the list members, and still do the walk_cb_nocall if the number of members does not go to zero after e.g. CXA_DTORS_ITERATIONS times initial list length for dtor removals.

In D55915#1281101, @timo.voelker_fh-muenster.de wrote:

In D55915#1280245, @kib wrote:

You only write a single value into the specified offset, right?
This should be clearly explained in the man page addition.
Or consider adding a possibility of writing the series of values starting at the given offset, of the specified width for each value (IMO it is fine as is, but needs to be properly documented).

In my use case, I only had to write a single value. Do you know use cases where a series of values needs to be written to a BAR region?

I tried to make it similar to -w (write to config space). This is the man page part about -w:

The -w option writes the value into a configuration space register at byte offset addr of device selector.

Do you find this more clear?

Handle all arches

Move the proto to apicvar.h.

In D56005#1281266, @ehem_freebsd_m5p.com wrote:

This is wrong. Specifically machine/xen/arch-intr.h is the MI <=> MD interface whereas xen_arch_intr_handle_upcall() is the x86 assembly-language <=> C interface. Mixing the two together would be bad.

It is C function, it can be called from C. What is the problem?

Use trapframe_fred

Not needed for the latest code.

In D55912#1280104, @gallatin wrote:

In D55912#1279758, @kib wrote:

Why not simply not allocate the new number for the new ioctl. Rename current ioctl to something like SIOCGI2C_NOBANK. Then, at the ioctl level, do two things:

• for new SIOCGI2C check that reserved bytes in the structure are zero
• for SIOCGI2C_NOBANK, zero bank/page/padding and call into SIOCGI2C

I think I'd rather go the other direction, so as to avoid touching legacy ioctl users from ports. It turns out there are many from the ports sweep.

Eg, we add SIOCGI2CPB (where PB means page/bank) for new code. Drivers like mlx5 which can support page and/or bank access add a fallthru to SIOCGI2CPB to their SIOCGI2C case and zero page & bank for SIOCGI2C. Eg:

	case SIOCGI2C:
	case SIOCGI2CPB:
		ifr = (struct ifreq *)data;

		/*
		 * Copy from the user-space address ifr_data to the
		 * kernel-space address i2c
		 */
		error = copyin(ifr_data_get_ptr(ifr), &i2c, sizeof(i2c));
		if (error)
			break;

		/* Esnure page & bank are zero for legacy SIOCGI2C ioctl */
		if (command == SIOCGI2C) {
			i2c.page = i2c.bank = 0;
		}

XENHVM upcall IPI handler

This should be functionally complete with the one minor missed XEN ipi handler code.

Missed check.

Centralize and fix checks.

You only write a single value into the specified offset, right?
This should be clearly explained in the man page addition.
Or consider adding a possibility of writing the series of values starting at the given offset, of the specified width for each value (IMO it is fine as is, but needs to be properly documented).

I only handled x86 for start. After everybody is fine with the patch, I will add other arches.

In D55831#1279764, @jhb wrote:

I can't recall if kgdb has to know the size of struct trapframe is my only concern. I think we already only use the 16 bits for cs/ss today.

Why not simply not allocate the new number for the new ioctl. Rename current ioctl to something like SIOCGI2C_NOBANK. Then, at the ioctl level, do two things:

• for new SIOCGI2C check that reserved bytes in the structure are zero
• for SIOCGI2C_NOBANK, zero bank/page/padding and call into SIOCGI2C

In D55931#1279683, @jhb wrote:

Speaking of this btw, it might be nice to fix other architectures to also pull the pcb out of the kstack in general. Certainly for CHERI we prefer that arrangement.

In D55831#1279681, @jhb wrote:

FYI, kgdb will have to know about the change in layout to trapframe I think, so bump __FreeBSD_version when you land this just to be on the safe side please?

Functionally complete patch

Continue the WIP.
Handle NMIs and #MC.
Misc bug fixes.

For now, I suggest to #ifdef 0 the ADD_TEST(), then revert it when the updated test is written.

In D55893#1278868, @aokblast wrote:

In D55893#1278865, @kib wrote:

No, I mean to keep this test, adjusted.
Create a global static var that is incremented on each again() call, and do not call __cxa_thread_atexit() if the counter greater than some value.
As an additional check, you might assert that the counter indeed counted to the specified value.

In D55893#1278865, @kib wrote:

No, I mean to keep this test, adjusted.
Create a global static var that is incremented on each again() call, and do not call __cxa_thread_atexit() if the counter greater than some value.
As an additional check, you might assert that the counter indeed counted to the specified value.

What about the nothr case? Since there is only one main thread. The dtos will be called after the ATF_TEST_CASE_BODY, which means we are unable to use assert since the main thread is destroyed.

No, I mean to keep this test, adjusted.
Create a global static var that is incremented on each again() call, and do not call __cxa_thread_atexit() if the counter greater than some value.
As an additional check, you might assert that the counter indeed counted to the specified value.

Did you tried enabling DMAR/AMD IOMMU and interrupt remapping on this machine?

In D55826#1278380, @aokblast wrote:

In D55826#1278375, @siva wrote:

@aokblast this commit breaks the following tests in CI:

lib/libc/stdlib/cxa_thread_atexit_nothr_test:cxx__thread_inf_dtors
lib/libc/stdlib/cxa_thread_atexit_test:cxx__thread_inf_dtors
lib/libc/stdlib/cxa_thread_atexit_test:cxx__thread_local_add_while_calling_dtors
lib/libc/stdlib/cxa_thread_atexit_test:cxx__thread_local_after
lib/libc/stdlib/cxa_thread_atexit_test:cxx__thread_local_before

See https://ci.freebsd.org/view/Test/job/FreeBSD-main-amd64-test/28044/testReport/.

I don't think the test actually make sense with my change.
The following code in the test makes it unables to terminate

static void                                                                                                                                        
again(void *arg)                                                                                                                                   
{                                                                                                                                                  
                                                                                                                                                   
    __cxa_thread_atexit(again, arg, &output);                                                                                                      
}

This makes again(void *) called himself and create a infinite loop. In my opnion, the user of the __cxa_thread_atexit should take the responsibility on the infinite loop instead of setting the hard limit on iteration.

I just take a look at the original commit which add this test and would like to ask @kib and @dim on their opinions.

Should we remove the again function or if there is any suggestions on this?

Fix typo in the name of the reserved field.
Remove cs masking.