svn commit: r368774 - in head/sys/dev/usb: . quirk
Author: jrtc27 Date: Fri Dec 18 23:31:36 2020 New Revision: 368774 URL: https://svnweb.freebsd.org/changeset/base/368774 Log: usb: Replace ITUNERNET vendor with MICROCHIP and improve product names These Mini-Box LCDs are using Microchip components and sub-licensed product IDs. Whilst here, update the constant names and descriptions for the products to use the names listed on the manufacturer's website rather than vague ones. The picoLCD 4x20 is named that on the manufacturer's website so prefer that name, even though linux-usb.org lists it with the numbers reversed as one might expect. Reviewed by: hselasky Differential Revision:https://reviews.freebsd.org/D27670 Modified: head/sys/dev/usb/quirk/usb_quirk.c head/sys/dev/usb/usbdevs Modified: head/sys/dev/usb/quirk/usb_quirk.c == --- head/sys/dev/usb/quirk/usb_quirk.c Fri Dec 18 23:28:27 2020 (r368773) +++ head/sys/dev/usb/quirk/usb_quirk.c Fri Dec 18 23:31:36 2020 (r368774) @@ -128,8 +128,8 @@ static struct usb_quirk_entry usb_quirks[USB_DEV_QUIRK USB_QUIRK(CYPRESS, SILVERSHIELD, 0x, 0x, UQ_HID_IGNORE), USB_QUIRK(DELORME, EARTHMATE, 0x, 0x, UQ_HID_IGNORE), USB_QUIRK(DREAMLINK, DL100B, 0x, 0x, UQ_HID_IGNORE), - USB_QUIRK(ITUNERNET, USBLCD2X20, 0x, 0x, UQ_HID_IGNORE), - USB_QUIRK(ITUNERNET, USBLCD4X20, 0x, 0x, UQ_HID_IGNORE), + USB_QUIRK(MICROCHIP, PICOLCD20X2, 0x, 0x, UQ_HID_IGNORE), + USB_QUIRK(MICROCHIP, PICOLCD4X20, 0x, 0x, UQ_HID_IGNORE), USB_QUIRK(LIEBERT, POWERSURE_PXT, 0x, 0x, UQ_HID_IGNORE), USB_QUIRK(LIEBERT2, PSI1000, 0x, 0x, UQ_HID_IGNORE), USB_QUIRK(LIEBERT2, POWERSURE_PSA, 0x, 0x, UQ_HID_IGNORE), Modified: head/sys/dev/usb/usbdevs == --- head/sys/dev/usb/usbdevsFri Dec 18 23:28:27 2020(r368773) +++ head/sys/dev/usb/usbdevsFri Dec 18 23:31:36 2020(r368774) @@ -184,7 +184,7 @@ vendor ITTCANON 0x04d1 ITT Canon vendor ALTEC 0x04d2 Altec Lansing vendor LSI 0x04d4 LSI vendor MENTORGRAPHICS 0x04d6 Mentor Graphics -vendor ITUNERNET 0x04d8 I-Tuner Networks +vendor MICROCHIP 0x04d8 Microchip Technology, Inc. vendor HOLTEK 0x04d9 Holtek Semiconductor, Inc. vendor PANASONIC 0x04da Panasonic (Matsushita) vendor HUANHSIN0x04dc Huan Hsin @@ -2647,9 +2647,9 @@ product ISSC ISSCBTA 0x1001 Bluetooth USB Adapter product ITEGNO WM1080A 0x1080 WM1080A GSM/GPRS modem product ITEGNO WM2080A 0x2080 WM2080A CDMA modem -/* Ituner networks products */ -product ITUNERNET USBLCD2X20 0x0002 USB-LCD 2x20 -product ITUNERNET USBLCD4X20 0xc001 USB-LCD 4x20 +/* Microchip Technology, Inc. products */ +product MICROCHIP PICOLCD20X2 0x0002 Mini-Box picoLCD 20x2 +product MICROCHIP PICOLCD4X20 0xc001 Mini-Box picoLCD 4x20 /* Jablotron products */ product JABLOTRON PC60B0x0001 PC-60B ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r368770 - head/lib/libc/string
Author: jrtc27
Date: Fri Dec 18 22:10:17 2020
New Revision: 368770
URL: https://svnweb.freebsd.org/changeset/base/368770
Log:
strerror.3: Fix whitespace issue introduced in r368714
MFC with: 368714
Modified:
head/lib/libc/string/strerror.3
Modified: head/lib/libc/string/strerror.3
==
--- head/lib/libc/string/strerror.3 Fri Dec 18 22:00:57 2020
(r368769)
+++ head/lib/libc/string/strerror.3 Fri Dec 18 22:10:17 2020
(r368770)
@@ -188,7 +188,7 @@ main(void)
perror("open()");
exit(1);
}
-printf("File descriptor: %d\en", fd);
+ printf("File descriptor: %d\en", fd);
return (0);
}
.Ed
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r368761 - head/sys/dev/virtio/mmio
Author: jrtc27 Date: Fri Dec 18 15:07:14 2020 New Revision: 368761 URL: https://svnweb.freebsd.org/changeset/base/368761 Log: virtio_mmio: Fix feature negotiation copy-paste issue in r361943 This caused us to write to the low half of the feature word twice, once with the high bits and once with the low bits. Common legacy device implementations seem to be fairly lenient about being able to write to the feature bits multiple times, but Arm's models use a stricter implementation that will ignore the second write. This fixes using vtnet(4) on those models. Reported by: Jean-Philippe Brucker Pointy hat: jrtc27 Modified: head/sys/dev/virtio/mmio/virtio_mmio.c Modified: head/sys/dev/virtio/mmio/virtio_mmio.c == --- head/sys/dev/virtio/mmio/virtio_mmio.c Fri Dec 18 12:40:19 2020 (r368760) +++ head/sys/dev/virtio/mmio/virtio_mmio.c Fri Dec 18 15:07:14 2020 (r368761) @@ -409,10 +409,10 @@ vtmmio_negotiate_features(device_t dev, uint64_t child vtmmio_describe_features(sc, "negotiated", features); - vtmmio_write_config_4(sc, VIRTIO_MMIO_HOST_FEATURES_SEL, 1); + vtmmio_write_config_4(sc, VIRTIO_MMIO_GUEST_FEATURES_SEL, 1); vtmmio_write_config_4(sc, VIRTIO_MMIO_GUEST_FEATURES, features >> 32); - vtmmio_write_config_4(sc, VIRTIO_MMIO_HOST_FEATURES_SEL, 0); + vtmmio_write_config_4(sc, VIRTIO_MMIO_GUEST_FEATURES_SEL, 0); vtmmio_write_config_4(sc, VIRTIO_MMIO_GUEST_FEATURES, features); return (features); ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r368738 - head/sys/compat/linuxkpi/common/include/linux
On 17 Dec 2020, at 20:28, John Baldwin wrote:
>
> Author: jhb
> Date: Thu Dec 17 20:28:53 2020
> New Revision: 368738
> URL: https://svnweb.freebsd.org/changeset/base/368738
>
> Log:
> Cleanups to *ERR* compat shims.
>
> - Use [u]intptr_t casts to convert pointers to integers.
>
> - Change IS_ERR* to return bool instead of long.
>
> Reviewed by: manu
> Obtained from: CheriBSD
> Sponsored by:DARPA
> Differential Revision: https://reviews.freebsd.org/D27577
>
> Modified:
> head/sys/compat/linuxkpi/common/include/linux/err.h
>
> Modified: head/sys/compat/linuxkpi/common/include/linux/err.h
> ==
> --- head/sys/compat/linuxkpi/common/include/linux/err.h Thu Dec 17
> 20:11:31 2020(r368737)
> +++ head/sys/compat/linuxkpi/common/include/linux/err.h Thu Dec 17
> 20:28:53 2020(r368738)
> @@ -37,30 +37,30 @@
>
> #define MAX_ERRNO 4095
>
> -#define IS_ERR_VALUE(x) unlikely((x) >= (unsigned long)-MAX_ERRNO)
> +#define IS_ERR_VALUE(x) unlikely((x) >= (uintptr_t)-MAX_ERRNO)
>
> static inline void *
> ERR_PTR(long error)
> {
> - return (void *)error;
> + return (void *)(intptr_t)error;
> }
>
> static inline long
> PTR_ERR(const void *ptr)
> {
> - return (long)ptr;
> + return (intptr_t)ptr;
This should probably be (long)(intptr_t) after no longer changing the
return type to intptr_t?
Jess
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r368714 - head/lib/libc/string
On 17 Dec 2020, at 16:22, Konstantin Belousov wrote:
> On Thu, Dec 17, 2020 at 01:01:01PM +0000, Jessica Clarke wrote:
>> On 17 Dec 2020, at 12:53, Konstantin Belousov wrote:
>>>
>>> On Thu, Dec 17, 2020 at 12:41:47PM +, Mateusz Piotrowski wrote:
>>>> Author: 0mp (doc,ports committer)
>>>> Date: Thu Dec 17 12:41:47 2020
>>>> New Revision: 368714
>>>> URL: https://svnweb.freebsd.org/changeset/base/368714
>>>>
>>>> Log:
>>>> strerror.3: Add an example for perror()
>>>>
>>>> This is a nice and quick reference.
>>>>
>>>> Reviewed by: jilles, yuripv
>>>> MFC after: 2 weeks
>>>> Differential Revision: https://reviews.freebsd.org/D27623
>>>>
>>>> Modified:
>>>> head/lib/libc/string/strerror.3
>>>>
>>>> Modified: head/lib/libc/string/strerror.3
>>>> ==
>>>> --- head/lib/libc/string/strerror.3Thu Dec 17 03:42:54 2020
>>>> (r368713)
>>>> +++ head/lib/libc/string/strerror.3Thu Dec 17 12:41:47 2020
>>>> (r368714)
>>>> @@ -32,7 +32,7 @@
>>>> .\" @(#)strerror.3 8.1 (Berkeley) 6/9/93
>>>> .\" $FreeBSD$
>>>> .\"
>>>> -.Dd December 7, 2020
>>>> +.Dd December 17, 2020
>>>> .Dt STRERROR 3
>>>> .Os
>>>> .Sh NAME
>>>> @@ -170,6 +170,31 @@ The use of these variables is deprecated;
>>>> or
>>>> .Fn strerror_r
>>>> should be used instead.
>>>> +.Sh EXAMPLES
>>>> +The following example shows how to use
>>>> +.Fn perror
>>>> +to report an error.
>>>> +.Bd -literal -offset 2n
>>>> +#include
>>>> +#include
>>>> +#include
>>>> +
>>>> +int
>>>> +main(void)
>>>> +{
>>>> + int fd;
>>>> +
>>>> + if ((fd = open("/nonexistent", O_RDONLY)) == -1) {
>>>> + perror("open()");
>>>> + exit(1);
>>>> + }
>>>> +printf("File descriptor: %d\en", fd);
>>> This lines is indented with spaces, while other lines have tabs.
>>>
>>>> + return (0);
>>> return (0) is redundand.
>>
>> It's not required as per the standard, but omitting it is needlessly
>> obfuscating it and bad practice. C lets you do a whole load of things
>> that are a bad idea, and whilst this one is harmless, it is nonetheless
>> confusing to anyone who is not intimately acquainted quirks like this
>> special case in the standard.
> Why it is bad practice ?
>
> C is a small language, and while knowing some quirks (like this one)
> seems to be optional, others are not. And worse, that other quirks are
> essential for writing correct code at all. Consequence is that ignoring
> details indicates insufficient knowledge of the fundamentals and lowers
> the trust in the provided suggestion.
Small does not mean simple. You admit that the language has quirks you
need to know in order to be able to write correct code, so why should
we confound the problem by forcing people to be aware of additional
optional quirks? This is just another kind of code golfing that
achieves nothing other than confuse some people (how many C courses do
you know of that actually teach you that you can omit the final return
statement from main?) and possibly mask a bug if you meant to return a
non-zero value (generally unlikely as error conditions will be handled
early, but maybe you have a final ret value lying around you meant to
return), when the alternative is just to add a single extra line to be
explicit about what you want. Especially since this is example code, we
should be seeking to provide as simple and clear code as is possible,
though I would not like to see this kind of code enter the base system
either.
Jess
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r368714 - head/lib/libc/string
On 17 Dec 2020, at 12:53, Konstantin Belousov wrote:
>
> On Thu, Dec 17, 2020 at 12:41:47PM +, Mateusz Piotrowski wrote:
>> Author: 0mp (doc,ports committer)
>> Date: Thu Dec 17 12:41:47 2020
>> New Revision: 368714
>> URL: https://svnweb.freebsd.org/changeset/base/368714
>>
>> Log:
>> strerror.3: Add an example for perror()
>>
>> This is a nice and quick reference.
>>
>> Reviewed by:jilles, yuripv
>> MFC after: 2 weeks
>> Differential Revision: https://reviews.freebsd.org/D27623
>>
>> Modified:
>> head/lib/libc/string/strerror.3
>>
>> Modified: head/lib/libc/string/strerror.3
>> ==
>> --- head/lib/libc/string/strerror.3 Thu Dec 17 03:42:54 2020
>> (r368713)
>> +++ head/lib/libc/string/strerror.3 Thu Dec 17 12:41:47 2020
>> (r368714)
>> @@ -32,7 +32,7 @@
>> .\" @(#)strerror.3 8.1 (Berkeley) 6/9/93
>> .\" $FreeBSD$
>> .\"
>> -.Dd December 7, 2020
>> +.Dd December 17, 2020
>> .Dt STRERROR 3
>> .Os
>> .Sh NAME
>> @@ -170,6 +170,31 @@ The use of these variables is deprecated;
>> or
>> .Fn strerror_r
>> should be used instead.
>> +.Sh EXAMPLES
>> +The following example shows how to use
>> +.Fn perror
>> +to report an error.
>> +.Bd -literal -offset 2n
>> +#include
>> +#include
>> +#include
>> +
>> +int
>> +main(void)
>> +{
>> +int fd;
>> +
>> +if ((fd = open("/nonexistent", O_RDONLY)) == -1) {
>> +perror("open()");
>> +exit(1);
>> +}
>> +printf("File descriptor: %d\en", fd);
> This lines is indented with spaces, while other lines have tabs.
>
>> +return (0);
> return (0) is redundand.
It's not required as per the standard, but omitting it is needlessly
obfuscating it and bad practice. C lets you do a whole load of things
that are a bad idea, and whilst this one is harmless, it is nonetheless
confusing to anyone who is not intimately acquainted quirks like this
special case in the standard.
Jess
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r368700 - head/sys/dev/e1000
Author: jrtc27
Date: Wed Dec 16 14:48:46 2020
New Revision: 368700
URL: https://svnweb.freebsd.org/changeset/base/368700
Log:
Fix whitespace in r368698
MFC with: r368698
Modified:
head/sys/dev/e1000/if_em.c
Modified: head/sys/dev/e1000/if_em.c
==
--- head/sys/dev/e1000/if_em.c Wed Dec 16 14:47:49 2020(r368699)
+++ head/sys/dev/e1000/if_em.c Wed Dec 16 14:48:46 2020(r368700)
@@ -847,7 +847,7 @@ em_if_attach_pre(if_ctx_t ctx)
** use a different BAR, so we need to keep
** track of which is used.
*/
- scctx->isc_msix_bar = pci_msix_table_bar(dev);
+ scctx->isc_msix_bar = pci_msix_table_bar(dev);
} else if (adapter->hw.mac.type >= em_mac_min) {
scctx->isc_txqsizes[0] = roundup2(scctx->isc_ntxd[0]*
sizeof(struct e1000_tx_desc), EM_DBA_ALIGN);
scctx->isc_rxqsizes[0] = roundup2(scctx->isc_nrxd[0] *
sizeof(union e1000_rx_desc_extended), EM_DBA_ALIGN);
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r368699 - head/sys/arm64/arm64
Author: jrtc27 Date: Wed Dec 16 14:47:49 2020 New Revision: 368699 URL: https://svnweb.freebsd.org/changeset/base/368699 Log: Fix whitespace in comment modified by r368697 Modified: head/sys/arm64/arm64/busdma_bounce.c Modified: head/sys/arm64/arm64/busdma_bounce.c == --- head/sys/arm64/arm64/busdma_bounce.cWed Dec 16 14:39:24 2020 (r368698) +++ head/sys/arm64/arm64/busdma_bounce.cWed Dec 16 14:47:49 2020 (r368699) @@ -442,7 +442,7 @@ bounce_bus_dmamap_create(bus_dma_tag_t dmat, int flags bz = dmat->bounce_zone; /* -* Attempt to add pages to our pool on a per-instancebasis up to a sane +* Attempt to add pages to our pool on a per-instance basis up to a sane * limit. Even if the tag isn't subject of bouncing due to alignment * and boundary constraints, it could still auto-bounce due to * cacheline alignment, which requires at most two bounce pages. ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r368667 - in head: . gnu/usr.bin gnu/usr.bin/binutils gnu/usr.bin/gdb tools/build/mk
On 15 Dec 2020, at 23:25, John Baldwin wrote:
> On 12/15/20 9:44 AM, Ed Maste wrote:
>> Author: emaste
>> Date: Tue Dec 15 17:44:19 2020
>> New Revision: 368667
>> URL: https://svnweb.freebsd.org/changeset/base/368667
>>
>> Log:
>> Retire obsolete GDB 6.1.1
>>
>> GDB 6.1.1 was released in June 2004 and is long obsolete. It does not
>> support all of the architectures that FreeBSD does, and imposes
>> limitations on the FreeBSD kernel build, such as the continued use of
>> DWARF2 debugging information.
>>
>> It was kept (in /usr/libexec/) only for use by crashinfo(8), which
>> extracts some basic information from a kernel core dump after a crash.
>> Crashinfo already prefers gdb from port/package if installed.
>>
>> Future work may add kernel debug support to LLDB or find another path
>> for crashinfo's needs, but in any case we do not want to ship the
>> excessively outdated GDB in FreeBSD 13.
>>
>> Sponsored by: The FreeBSD Foundation
>> Differential Revision: https://reviews.freebsd.org/D27610
>
> Are you going to remove the -gdwarf-2 bits from kern.mk now?
>
> (Does ctfconvert support newer DWARF?)
cddl/contrib/opensolaris/tools/ctf/cvt/dwarf.c:1964-1968:
debug(1, "DWARF version: %d\n", vers);
if (vers < 2 || vers > 4) {
terminate("file contains incompatible version %d DWARF code "
"(version 2, 3 or 4 required)\n", vers);
}
(since r261025)
Though that doesn't mean it works...
Jess
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r368626 - head/stand/efi/loader/arch/riscv
Author: jrtc27
Date: Mon Dec 14 00:54:05 2020
New Revision: 368626
URL: https://svnweb.freebsd.org/changeset/base/368626
Log:
loader: Ignore the .interp section on RISC-V
Without this we risk having the .interp section be placed earlier in the
file and mess with section offsets; in particular it has been seen to be
placed at the start of the file and cause the PE/COFF header to not be
at address 0. This is the same fix as was done for arm64 in r365578.
Reviewed by: mhorne, imp
Approved by: mhorne, imp
Differential Revision:https://reviews.freebsd.org/D27603
Modified:
head/stand/efi/loader/arch/riscv/ldscript.riscv
Modified: head/stand/efi/loader/arch/riscv/ldscript.riscv
==
--- head/stand/efi/loader/arch/riscv/ldscript.riscv Mon Dec 14 00:50:45
2020(r368625)
+++ head/stand/efi/loader/arch/riscv/ldscript.riscv Mon Dec 14 00:54:05
2020(r368626)
@@ -82,6 +82,7 @@ SECTIONS
_edata = .;
/* Unused sections */
+ .interp : { *(.interp) }
.dynstr : { *(.dynstr) }
.hash: { *(.hash) }
}
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r368625 - head/lib/libc/string
Author: jrtc27 Date: Mon Dec 14 00:50:45 2020 New Revision: 368625 URL: https://svnweb.freebsd.org/changeset/base/368625 Log: strdup.3: Function appeared in 4.3BSD-Reno, not 4.4BSD Linux claims 4.3BSD, we claim 4.4BSD and OpenBSD claims 4.3BSD-Reno. It turns out that OpenBSD got it right: the function was added in late 1988 a few months after 4.3BSD-Tahoe, well in advance of 4.3BSD-Reno. Reviewed by: bcr Approved by: bcr Differential Revision:https://reviews.freebsd.org/D27392 Modified: head/lib/libc/string/strdup.3 Modified: head/lib/libc/string/strdup.3 == --- head/lib/libc/string/strdup.3 Mon Dec 14 00:47:59 2020 (r368624) +++ head/lib/libc/string/strdup.3 Mon Dec 14 00:50:45 2020 (r368625) @@ -91,7 +91,7 @@ function is specified by The .Fn strdup function first appeared in -.Bx 4.4 . +.Bx 4.3 Reno . The .Fn strndup function was added in ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r368624 - head/sys/mips/mips
Author: jrtc27 Date: Mon Dec 14 00:47:59 2020 New Revision: 368624 URL: https://svnweb.freebsd.org/changeset/base/368624 Log: mips: Fix sub-word atomics implementation These aligned the address but then always used the least significant bits of the value in memory, which is the wrong half 50% of the time for 16-bit atomics and the wrong quarter 75% of the time for 8-bit atomics. These bugs were all present in r178172, the commit that added the mips port, and have remained for its entire existence to date. Reviewed by: jhb (mentor) Approved by: jhb (mentor) Differential Revision:https://reviews.freebsd.org/D27343 Modified: head/sys/mips/mips/support.S Modified: head/sys/mips/mips/support.S == --- head/sys/mips/mips/support.SMon Dec 14 00:46:24 2020 (r368623) +++ head/sys/mips/mips/support.SMon Dec 14 00:47:59 2020 (r368624) @@ -90,6 +90,7 @@ #include #include #include +#include #include #include #include @@ -578,9 +579,14 @@ END(ffs) */ LEAF(atomic_set_16) .setnoreorder - srl a0, a0, 2 # round down address to be 32-bit aligned - sll a0, a0, 2 - andia1, a1, 0x + /* NB: Only bit 1 is masked so the ll catches unaligned inputs */ + andit0, a0, 2 # get unaligned offset + xor a0, a0, t0 # align pointer +#if _BYTE_ORDER == BIG_ENDIAN + xorit0, t0, 2 +#endif + sll t0, t0, 3 # convert byte offset to bit offset + sll a1, a1, t0 # put bits in the right half 1: ll t0, 0(a0) or t0, t0, a1 @@ -600,17 +606,18 @@ END(atomic_set_16) */ LEAF(atomic_clear_16) .setnoreorder - srl a0, a0, 2 # round down address to be 32-bit aligned - sll a0, a0, 2 - nor a1, zero, a1 + /* NB: Only bit 1 is masked so the ll catches unaligned inputs */ + andit0, a0, 2 # get unaligned offset + xor a0, a0, t0 # align pointer +#if _BYTE_ORDER == BIG_ENDIAN + xorit0, t0, 2 +#endif + sll t0, t0, 3 # convert byte offset to bit offset + sll a1, a1, t0 # put bits in the right half + not a1, a1 1: ll t0, 0(a0) - movet1, t0 - andit1, t1, 0x # t1 has the original lower 16 bits - and t1, t1, a1 # t1 has the new lower 16 bits - srl t0, t0, 16 # preserve original top 16 bits - sll t0, t0, 16 - or t0, t0, t1 + and t0, t0, a1 sc t0, 0(a0) beq t0, zero, 1b nop @@ -628,17 +635,23 @@ END(atomic_clear_16) */ LEAF(atomic_subtract_16) .setnoreorder - srl a0, a0, 2 # round down address to be 32-bit aligned - sll a0, a0, 2 + /* NB: Only bit 1 is masked so the ll catches unaligned inputs */ + andit0, a0, 2 # get unaligned offset + xor a0, a0, t0 # align pointer +#if _BYTE_ORDER == BIG_ENDIAN + xorit0, t0, 2 # flip order for big-endian +#endif + sll t0, t0, 3 # convert byte offset to bit offset + sll a1, a1, t0 # put bits in the right half + li t2, 0x + sll t2, t2, t0 # compute mask 1: ll t0, 0(a0) - movet1, t0 - andit1, t1, 0x # t1 has the original lower 16 bits - subut1, t1, a1 - andit1, t1, 0x # t1 has the new lower 16 bits - srl t0, t0, 16 # preserve original top 16 bits - sll t0, t0, 16 - or t0, t0, t1 + subut1, t0, a1 + /* Exploit ((t0 & ~t2) | (t1 & t2)) = t0 ^ ((t0 ^ t1) & t2) */ + xor t1, t0, t1 + and t1, t1, t2 + xor t0, t0, t1 sc t0, 0(a0) beq t0, zero, 1b nop @@ -655,17 +668,23 @@ END(atomic_subtract_16) */ LEAF(atomic_add_16) .setnoreorder - srl a0, a0, 2 # round down address to be 32-bit aligned - sll a0, a0, 2 + /* NB: Only bit 1 is masked so the ll catches unaligned inputs */ + andit0, a0, 2 # get unaligned offset + xor a0, a0, t0 # align pointer +#if _BYTE_ORDER == BIG_ENDIAN + xorit0, t0, 2 # flip order for big-endian +#endif + sll t0, t0, 3 # convert byte offset to bit offset + sll a1, a1, t0 # put bits in the right half + li t2, 0x + sll t2, t2, t0 # compute mask 1: ll t0, 0(a0) - movet1, t0 - andit1, t1, 0x # t1 has the original lower 16 bits - addut1, t1, a1 - andit1, t1, 0x # t1 has the new lower 16 bits - srl t0, t0, 16 # preserve original top 16 bits - sll t0, t0, 16 - or t0, t0, t1 +
svn commit: r368623 - head/stand/common
Author: jrtc27
Date: Mon Dec 14 00:46:24 2020
New Revision: 368623
URL: https://svnweb.freebsd.org/changeset/base/368623
Log:
loader: Print autoboot countdown immediately, not at 9
For the first second otime and ntime are equal so no message gets
printed. Instead we should print the countdown right from the start,
although we do it at the end of the first iteration so that if a key has
already been pressed then the message is suppressed.
Reviewed by: imp
Approved by: imp
Differential Revision:https://reviews.freebsd.org/D26935
Modified:
head/stand/common/boot.c
Modified: head/stand/common/boot.c
==
--- head/stand/common/boot.cSun Dec 13 23:51:51 2020(r368622)
+++ head/stand/common/boot.cMon Dec 14 00:46:24 2020(r368623)
@@ -202,8 +202,9 @@ autoboot(int timeout, char *prompt)
}
if (timeout >= 0) {
- otime = time(NULL);
- when = otime + timeout; /* when to boot */
+ otime = -1;
+ ntime = time(NULL);
+ when = ntime + timeout; /* when to boot */
yes = 0;
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r368410 - head/stand/libsa/zfs
On 7 Dec 2020, at 11:25, Toomas Soome wrote:
> @@ -183,28 +215,29 @@ xdr_u_int(xdr_t *xdr, unsigned *ip)
> static bool
> xdr_int64(xdr_t *xdr, int64_t *lp)
> {
> - int hi;
> - unsigned lo;
> bool rv = false;
>
> - if (xdr->xdr_idx + sizeof (int64_t) > xdr->xdr_buf + xdr->xdr_buf_size)
> + if (xdr->xdr_idx + sizeof(int64_t) > xdr->xdr_buf + xdr->xdr_buf_size)
> return (rv);
>
> switch (xdr->xdr_op) {
> case XDR_OP_ENCODE:
> /* Encode value *lp, store to buf */
> - hi = *lp >> 32;
> - lo = *lp & UINT32_MAX;
> - xdr->xdr_idx += xdr->xdr_putint(xdr, hi);
> - xdr->xdr_idx += xdr->xdr_putint(xdr, lo);
> + if (xdr->xdr_putint == _putint)
> + *(int64_t *)xdr->xdr_idx = htobe64(*lp);
> + else
> + *(int64_t *)xdr->xdr_idx = *lp;
I don't know the details here, but inspecting the callback function and
comparing it against a known one to decide what to do is generally not
good practice. Can this be pushed down into the function in question,
up into the caller or additional information passed to be explicit
about this behaviour rather than brittle magic behaviour?
Jess
Jess
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r368375 - head/sys/kern
Hi Mateusz,
This looks like a behavioural change to me. Was that intended and a bug
fix (in which case, it should have been documented in the commit
message) or not? See below for the exact change.
On 6 Dec 2020, at 04:59, Mateusz Guzik wrote:
> +static int
> +namei_getpath(struct nameidata *ndp)
> +{
> + struct componentname *cnp;
> + int error;
> +
> + cnp = >ni_cnd;
> +
> + /*
> + * Get a buffer for the name to be translated, and copy the
> + * name into the buffer.
> + */
> + cnp->cn_pnbuf = uma_zalloc(namei_zone, M_WAITOK);
> + if (ndp->ni_segflg == UIO_SYSSPACE) {
> + error = copystr(ndp->ni_dirp, cnp->cn_pnbuf, MAXPATHLEN,
> + >ni_pathlen);
> + } else {
> + error = copyinstr(ndp->ni_dirp, cnp->cn_pnbuf, MAXPATHLEN,
> + >ni_pathlen);
> + }
> +
> + if (__predict_false(error != 0)) {
> + return (error);
This does not call namei_cleanup_cnp.
> @@ -531,31 +568,11 @@ namei(struct nameidata *ndp)
> ndp->ni_lcf = 0;
> ndp->ni_vp = NULL;
>
> - /*
> - * Get a buffer for the name to be translated, and copy the
> - * name into the buffer.
> - */
> - cnp->cn_pnbuf = uma_zalloc(namei_zone, M_WAITOK);
> - if (ndp->ni_segflg == UIO_SYSSPACE)
> - error = copystr(ndp->ni_dirp, cnp->cn_pnbuf, MAXPATHLEN,
> - >ni_pathlen);
> - else
> - error = copyinstr(ndp->ni_dirp, cnp->cn_pnbuf, MAXPATHLEN,
> - >ni_pathlen);
> -
> + error = namei_getpath(ndp);
> if (__predict_false(error != 0)) {
> - namei_cleanup_cnp(cnp);
But it used to be called in that case here.
> return (error);
> }
Jess
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r367813 - head/lib/libutil
On 18 Nov 2020, at 22:32, Stefan Esser wrote:
>
> Am 18.11.20 um 22:40 schrieb Mateusz Guzik:
>>> +{
>>> + static const int localbase_oid[2] = {CTL_USER, USER_LOCALBASE};
>> There is no use for this to be static.
>
> Why not? This makes it part of the constant initialized data of
> the library, which is more efficient under run-time and memory
> space aspects than initializing them on the stack.
>
> What do I miss?
What is more efficient is not so clear-cut, it depends on things like
the architecture, microarchitecture and ABI. Allocating a small buffer
on the stack is extremely cheap (the memory will almost certainly be in
the L1 cache), whereas globally-allocated storage is less likely to be
in the cache due to being spread out, and on some architecture/ABI
combinations will incur additional indirection through the GOT. Also, 8
bytes of additional stack use is lost in the noise.
Jess
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r367813 - head/lib/libutil
On 18 Nov 2020, at 21:52, Stefan Esser wrote: > Am 18.11.20 um 22:15 schrieb Jessica Clarke: >> On 18 Nov 2020, at 19:44, Stefan Eßer wrote: >>> + /* >>> +* Check for some other thread already having >>> +* set localbase - this should use atomic ops. >>> +* The amount of memory allocated above may leak, >>> +* if a parallel update in another thread is not >>> +* detected and the non-NULL pointer is overwritten. >>> +*/ >> Why was this committed with a known racy/leaky implementation? > > Because the alternatives that I offered for discussion were > less acceptable. That has no bearing over whether this one is. >> What happens if I set the value with a sysctl and call this? > > You'll get the value set with sysctl, unless overridden by the > environment variable. There is a window of a few nano-seconds > where a thread executing in parallel on another core might be > able to set the localbase variable (between the test for NULL > in this function and the assignment to it). The value that will > be returned by either thread will be identical, so there is no > risk of corruption of the result. But if I call getlocalbase, then set it via sysctl, then call getlocalbase again, I see the old value. If, however, I omit the first getlocalbase, I see the new value. This differs from how getenv/setenv of the value work, where you always see the up-to-date value. Maybe you think that's a feature, but it's something to watch out for and explicitly call out in the documentation. You also misunderstand all the subtleties of multithreading here. There are no acquire/release pairs so it is entirely legal for Thread 2 to read Thread 1's initialised value for localbase before the contents of it are visible (i.e. the pointer is initialised but the data is garbage). The `(volatile const char*)localbase` cast is also a complete waste of time. You probably meant to write `(const char * volatile)localbase` but even then that does nothing useful as the cast is too late. What you really were trying to write was `*(const char * volatile *)`, but you need proper atomics anyway for this to be safe. > This unlikely case may actually leak a heap allocated string > of typically tens of bytes (but with negligible probability). > > But this really is a non-issue, since there should never be a > reason to invoke this function in a multi-threaded context. Why not? There could easily be code out there calling getenv in a multi-threaded context so this is inadequate as a replacement. Yes it's inefficient but it's perfectly legal and imaginable. Also if malloc returns NULL I would quite like that to be an error for the function and not silently fall back on _PATH_LOCALBASE. > The result should be constant for the duration of execution > of the process (expect severe inconsistencies if that was not > the case) and all programs in base that are candidates for the > use of this function are non-threaded (and if they were multi- > threaded, then I'd expect this call to occur during start-up > of the program before any further threads are created). > > So, this is a non-issue and the comment tries to explain it. > Did I fail to make this clear in the comment? Maybe I should > have written "could use atomic ops" instead? > > Use of atomics or locks could prevent the race-condition. But > since I do not expect this function to be called from within > threads (it just doesn't make sense), the tiny time window of > a few nano-seconds which might lead to a double assignment to > the target variable (with one pointer value being lost), and > the worst case loss of 1KB of heap space in that case (more > likely 10 to 20 bytes rounded up to a 16 or 32 byte chunk), I > do not consider the complexities of either a lock or atomic ops > to be justified. > > Regards, STefan > ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r367813 - head/lib/libutil
On 18 Nov 2020, at 21:40, Mateusz Guzik wrote:
>
> On 11/18/20, Stefan Eßer wrote:
>> Author: se
>> Date: Wed Nov 18 19:44:30 2020
>> New Revision: 367813
>> URL: https://svnweb.freebsd.org/changeset/base/367813
>>
>> Log:
>> Add function getlocalbase() to libutil.
>>
>> This function returns the path to the local software base directory, by
>> default "/usr/local" (or the value of _PATH_LOCALBASE in include/paths.h
>> when building the world).
>>
>> The value returned can be overridden by 2 methods:
>>
>> - the LOCALBASE environment variable (ignored by SUID programs)
>> - else a non-default user.localbase sysctl value
>>
>> Reviewed by:hps (earlier version)
>> Relnotes: yes
>> Differential Revision: https://reviews.freebsd.org/D27236
>>
>> Added:
>> head/lib/libutil/getlocalbase.3 (contents, props changed)
>> head/lib/libutil/getlocalbase.c (contents, props changed)
>> Modified:
>> head/lib/libutil/Makefile
>> head/lib/libutil/libutil.h
>>
>> Modified: head/lib/libutil/Makefile
>> ==
>> --- head/lib/libutil/MakefileWed Nov 18 19:35:30 2020
>> (r367812)
>> +++ head/lib/libutil/MakefileWed Nov 18 19:44:30 2020
>> (r367813)
>> @@ -12,7 +12,8 @@ PACKAGE= runtime
>> LIB= util
>> SHLIB_MAJOR= 9
>>
>> -SRCS= _secure_path.c auth.c expand_number.c flopen.c fparseln.c
>> gr_util.c
>> \
>> +SRCS= _secure_path.c auth.c expand_number.c flopen.c fparseln.c \
>> +getlocalbase.c gr_util.c \
>> hexdump.c humanize_number.c kinfo_getfile.c \
>> kinfo_getallproc.c kinfo_getproc.c kinfo_getvmmap.c \
>> kinfo_getvmobject.c kld.c \
>> @@ -30,7 +31,7 @@ CFLAGS+= -DINET6
>>
>> CFLAGS+= -I${.CURDIR} -I${SRCTOP}/lib/libc/gen/
>>
>> -MAN+= expand_number.3 flopen.3 fparseln.3 hexdump.3 \
>> +MAN+= expand_number.3 flopen.3 fparseln.3 getlocalbase.3 hexdump.3 \
>> humanize_number.3 kinfo_getallproc.3 kinfo_getfile.3 \
>> kinfo_getproc.3 kinfo_getvmmap.3 kinfo_getvmobject.3 kld.3 \
>> login_auth.3 login_cap.3 \
>>
>> Added: head/lib/libutil/getlocalbase.3
>> ==
>> --- /dev/null00:00:00 1970 (empty, because file is newly added)
>> +++ head/lib/libutil/getlocalbase.3 Wed Nov 18 19:44:30 2020
>> (r367813)
>> @@ -0,0 +1,99 @@
>> +.\"
>> +.\" SPDX-License-Identifier: BSD-2-Clause-FreeBSD
>> +.\"
>> +.\" Copyright 2020 Scott Long
>> +.\" Copyright 2020 Stefan Eßer
>> +.\"
>> +.\" Redistribution and use in source and binary forms, with or without
>> +.\" modification, are permitted provided that the following conditions
>> +.\" are met:
>> +.\" 1. Redistributions of source code must retain the above copyright
>> +.\"notice, this list of conditions and the following disclaimer.
>> +.\" 2. Redistributions in binary form must reproduce the above copyright
>> +.\"notice, this list of conditions and the following disclaimer in the
>> +.\"documentation and/or other materials provided with the
>> distribution.
>> +.\"
>> +.\" THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
>> +.\" ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
>> +.\" IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
>> PURPOSE
>> +.\" ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE
>> LIABLE
>> +.\" FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
>> CONSEQUENTIAL
>> +.\" DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE
>> GOODS
>> +.\" OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
>> +.\" HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
>> STRICT
>> +.\" LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY
>> WAY
>> +.\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
>> +.\" SUCH DAMAGE.
>> +.\"
>> +.\" $FreeBSD$
>> +.\"
>> +.Dd November 18, 2020
>> +.Dt GETLOCALBASE 3
>> +.Os
>> +.Sh NAME
>> +.Nm getlocalbase
>> +.Nd "return the path to the local software directory"
>> +.Sh LIBRARY
>> +.Lb libutil
>> +.Sh SYNOPSIS
>> +.In libutil.h
>> +.Ft const char*
>> +.Fn getlocalbase "void"
>> +.Sh DESCRIPTION
>> +The
>> +.Fn getlocalbase
>> +function returns the path to the local software base directory.
>> +Normally this is the
>> +.Pa /usr/local
>> +directory.
>> +First the
>> +.Ev LOCALBASE
>> +environment variable is checked.
>> +If that does not exist then the
>> +.Va user.localbase
>> +sysctl is checked.
>> +If that also does not exist then the value of the
>> +.Dv _PATH_LOCALBASE
>> +compile-time variable is used.
>> +If that is undefined then the default of
>> +.Pa /usr/local
>> +is used.
>> +.Pp
>> +The value returned by the
>> +.Fn getlocalbase
>> +function shall not be modified.
>> +.Sh IMPLEMENTATION NOTES
>> +Calls to
>> +.Fn getlocalbase
>> +will perform a setugid
Re: svn commit: r367813 - head/lib/libutil
On 18 Nov 2020, at 21:15, Jessica Clarke wrote: > > On 18 Nov 2020, at 19:44, Stefan Eßer wrote: >> +/* >> + * Check for some other thread already having >> + * set localbase - this should use atomic ops. >> + * The amount of memory allocated above may leak, >> + * if a parallel update in another thread is not >> + * detected and the non-NULL pointer is overwritten. >> + */ > > Why was this committed with a known racy/leaky implementation? > > What happens if I set the value with a sysctl and call this? Notably, you go to all this trouble to have a localbase variable that gets set, but you never actually use it properly as a cache since you do the full lookup and only then realise that you already had a (possibly stale) value cached. Jess ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r367813 - head/lib/libutil
On 18 Nov 2020, at 19:44, Stefan Eßer wrote: > + /* > + * Check for some other thread already having > + * set localbase - this should use atomic ops. > + * The amount of memory allocated above may leak, > + * if a parallel update in another thread is not > + * detected and the non-NULL pointer is overwritten. > + */ Why was this committed with a known racy/leaky implementation? What happens if I set the value with a sysctl and call this? Jess > + if (tmppath[0] != '\0' && > + (volatile const char*)localbase == NULL) > + localbase = tmppath; > + else > + free((void*)tmppath); > + return (localbase); > + } > + return (_PATH_LOCALBASE); > +} ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r367771 - head/sbin/nvmecontrol
On 17 Nov 2020, at 17:29, Jessica Clarke wrote:
> On 17 Nov 2020, at 17:12, Adrian Chadd wrote:
>> @@ -175,8 +176,7 @@ update_firmware(int fd, uint8_t *payload, int32_t payl
>> errx(EX_OSERR, "unable to malloc %zd bytes",
>> (size_t)max_xfer_size);
>>
>> while (resid > 0) {
>> -size = (resid >= (int32_t)max_xfer_size) ?
>> -max_xfer_size : resid;
>> +size = (resid >= max_xfer_size) ? max_xfer_size : resid;
>
> MIN from the already-included sys/param.h?
>
> (Otherwise: you have an extra space after ?)
Also why is off signed when it counts up from 0?
Jess
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r367771 - head/sbin/nvmecontrol
On 17 Nov 2020, at 17:12, Adrian Chadd wrote:
> @@ -175,8 +176,7 @@ update_firmware(int fd, uint8_t *payload, int32_t payl
> errx(EX_OSERR, "unable to malloc %zd bytes",
> (size_t)max_xfer_size);
>
> while (resid > 0) {
> - size = (resid >= (int32_t)max_xfer_size) ?
> - max_xfer_size : resid;
> + size = (resid >= max_xfer_size) ? max_xfer_size : resid;
MIN from the already-included sys/param.h?
(Otherwise: you have an extra space after ?)
Jess
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r367701 - head/lib/libutil
On 15 Nov 2020, at 19:10, Brandon Bergren wrote: > > On powerpc64 and powerpc64le, there is some really weird behavior happening > around the sysctl itself: > > root@crow:~ # pkg > The package management tool is not yet installed on your system. > Do you want to fetch and install it now? [y/N]: N > root@crow:~ # sysctl user.localbase > user.localbase: /usr/local > root@crow:~ # pkg > The package management tool is not yet installed on your system. > Do you want to fetch and install it now? [y/N]: N > root@crow:~ # sysctl user.localbase=/usr/local > user.localbase: /usr/local -> /usr/local > root@crow:~ # pkg > pkg: not enough arguments > Usage: pkg [-v] [-d] [-l] [-N] [-j |-c |-r > ] [-C ] [-R ] [-o var=value] > [-4|-6] [] > > For more information on available commands and options see 'pkg help'. > root@crow:~ # > > > I would double check very closely that the sysctl is being called correctly, > the sysctl tool manages to read it out, but libutil does not. That's odd. What does truss say? Jess > On Sun, Nov 15, 2020, at 1:06 PM, Scott Long wrote: >> >>> On Nov 15, 2020, at 12:01 PM, Jessica Clarke wrote: >>>> >>>> I felt similar concerns, but my misunderstanding of strlcpy() drove the >>>> result. Since the use case for getlocalbase() lends itself to also use >>>> strlcat()/strlcpy(), I was trying to replicate the API semantics of those, >>>> at least to the limit of my understanding. Thanks for the feedback, I’ll >>>> look at it some more. >>> >>> Thanks. ENOMEM also feels inappropriate as no allocation is taking >>> place. Perhaps ENAMETOOLONG, which is used in similar cases for things >>> like gethostbyname? Though sysctlbyname uses ENOMEM instead... sigh. >>> >> >> Yep, I wasn’t happy with ENOMEM either but I couldn’t find anything better. >> >>> Also, if pathlen has already been checked against SSIZE_MAX (giving >>> EINVAL) and tmplen against pathlen there's no need to then check tmplen >>> against SSIZE_MAX. >>> >> >> Done. >> >>> I'd be happy to give a review on Phabricator if/when you have a new >>> patch. >>> >> >> https://reviews.freebsd.org/D27227 >> >> Thanks, >> Scott >> >> > > -- > Brandon Bergren > bdra...@freebsd.org ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r367701 - head/lib/libutil
On 15 Nov 2020, at 18:46, Scott Long wrote:
>> On Nov 15, 2020, at 11:31 AM, Jessica Clarke wrote:
>>
>> Hi Scott,
>> I'm concerned by this diff; see my comments below.
>>
>>> On 15 Nov 2020, at 07:48, Scott Long wrote:
>>>
>>> Author: scottl
>>> Date: Sun Nov 15 07:48:52 2020
>>> New Revision: 367701
>>> URL: https://svnweb.freebsd.org/changeset/base/367701
>>>
>>> Log:
>>> Because getlocalbase() returns -1 on error, it needs to use a signed type
>>> internally. Do that, and make sure that conversations between signed and
>>> unsigned don't overflow
>>>
>>> Modified:
>>> head/lib/libutil/getlocalbase.c
>>>
>>> Modified: head/lib/libutil/getlocalbase.c
>>> ==
>>> --- head/lib/libutil/getlocalbase.c Sun Nov 15 01:54:44 2020
>>> (r367700)
>>> +++ head/lib/libutil/getlocalbase.c Sun Nov 15 07:48:52 2020
>>> (r367701)
>>> @@ -41,7 +41,7 @@ __FBSDID("$FreeBSD$");
>>> ssize_t
>>> getlocalbase(char *path, size_t pathlen)
>>> {
>>> - size_t tmplen;
>>> + ssize_t tmplen;
>>> const char *tmppath;
>>>
>>> if ((pathlen == 0) || (path == NULL)) {
>>> @@ -49,13 +49,20 @@ getlocalbase(char *path, size_t pathlen)
>>> return (-1);
>>> }
>>>
>>> + /* It's unlikely that the buffer would be this big */
>>> + if (pathlen > SSIZE_MAX) {
>>> + errno = ENOMEM;
>>> + return (-1);
>>> + }
>>> +
>>> tmppath = NULL;
>>> - tmplen = pathlen;
>>> + tmplen = (size_t)pathlen;
>>> if (issetugid() == 0)
>>> tmppath = getenv("LOCALBASE");
>>>
>>> if ((tmppath == NULL) &&
>>> - (sysctlbyname("user.localbase", path, , NULL, 0) == 0)) {
>>> + (sysctlbyname("user.localbase", path, (size_t *), NULL,
>>
>> This is dangerous and generally a sign of something wrong.
>
> I think that the danger was mitigated by the overflow check above, but I
> agree that this is generally a poor pattern.
>
>>
>>> + 0) == 0)) {
>>> return (tmplen);
>>> }
>>>
>>> @@ -67,13 +74,13 @@ getlocalbase(char *path, size_t pathlen)
>>> #endif
>>>
>>> tmplen = strlcpy(path, tmppath, pathlen);
>>> - if ((tmplen < 0) || (tmplen >= pathlen)) {
>>> + if ((tmplen < 0) || (tmplen >= (ssize_t)pathlen)) {
>>
>> As I commented on the previous commit (but which you do not appear to
>> have picked up on), the LHS is impossible. Of course, now the types
>> have changed so the compiler doesn't know that.
>>
>
> The man page for strlcpy() made reference to the return value being
> equivalent to what snprintf() does. The man page for snprintf() states
> that negatve return values are possible, so I assumed the same was
> true for strlcpy(). However, now that I’ve looked at the implementation
> of strlcpy(), I see that you’re correct. The man pages are definitely
> confusing, and this isn’t the only place where I think there’s
> inconsistency in the documentation, or at least poor wording choices.
>
>> I think tmplen should have remained a size_t, as everywhere it's used
>> you're having to cast, which is generally a sign you've done something
>> wrong. Only when you come to return from the function do you need a
>> single cast to ssize_t (provided you've checked for overflow first). I
>> strongly believe this entire commit should be reverted, and whatever
>> bug you were trying to fixed be fixed in another way without using the
>> wrong types and adding an array of unnecessary and/or dangerous casts.
>>
>
> I felt similar concerns, but my misunderstanding of strlcpy() drove the
> result. Since the use case for getlocalbase() lends itself to also use
> strlcat()/strlcpy(), I was trying to replicate the API semantics of those,
> at least to the limit of my understanding. Thanks for the feedback, I’ll
> look at it some more.
Thanks. ENOMEM also feels inappropriate as no allocation is taking
place. Perhaps ENAMETOOLONG, which is used in similar cases for things
like gethostbyname? Though sysctlbyname uses ENOMEM instead... sigh.
Also, if pathlen has already been checked against SSIZE_MAX (giving
EINVAL) and tmplen against pathlen there's no need to then check tmplen
against SSIZE_MAX.
I'd be happy to give a review on Phabricator if/when you have a new
patch.
Jess
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r367701 - head/lib/libutil
Hi Scott,
I'm concerned by this diff; see my comments below.
> On 15 Nov 2020, at 07:48, Scott Long wrote:
>
> Author: scottl
> Date: Sun Nov 15 07:48:52 2020
> New Revision: 367701
> URL: https://svnweb.freebsd.org/changeset/base/367701
>
> Log:
> Because getlocalbase() returns -1 on error, it needs to use a signed type
> internally. Do that, and make sure that conversations between signed and
> unsigned don't overflow
>
> Modified:
> head/lib/libutil/getlocalbase.c
>
> Modified: head/lib/libutil/getlocalbase.c
> ==
> --- head/lib/libutil/getlocalbase.c Sun Nov 15 01:54:44 2020
> (r367700)
> +++ head/lib/libutil/getlocalbase.c Sun Nov 15 07:48:52 2020
> (r367701)
> @@ -41,7 +41,7 @@ __FBSDID("$FreeBSD$");
> ssize_t
> getlocalbase(char *path, size_t pathlen)
> {
> - size_t tmplen;
> + ssize_t tmplen;
> const char *tmppath;
>
> if ((pathlen == 0) || (path == NULL)) {
> @@ -49,13 +49,20 @@ getlocalbase(char *path, size_t pathlen)
> return (-1);
> }
>
> + /* It's unlikely that the buffer would be this big */
> + if (pathlen > SSIZE_MAX) {
> + errno = ENOMEM;
> + return (-1);
> + }
> +
> tmppath = NULL;
> - tmplen = pathlen;
> + tmplen = (size_t)pathlen;
> if (issetugid() == 0)
> tmppath = getenv("LOCALBASE");
>
> if ((tmppath == NULL) &&
> - (sysctlbyname("user.localbase", path, , NULL, 0) == 0)) {
> + (sysctlbyname("user.localbase", path, (size_t *), NULL,
This is dangerous and generally a sign of something wrong.
> + 0) == 0)) {
> return (tmplen);
> }
>
> @@ -67,13 +74,13 @@ getlocalbase(char *path, size_t pathlen)
> #endif
>
> tmplen = strlcpy(path, tmppath, pathlen);
> - if ((tmplen < 0) || (tmplen >= pathlen)) {
> + if ((tmplen < 0) || (tmplen >= (ssize_t)pathlen)) {
As I commented on the previous commit (but which you do not appear to
have picked up on), the LHS is impossible. Of course, now the types
have changed so the compiler doesn't know that.
I think tmplen should have remained a size_t, as everywhere it's used
you're having to cast, which is generally a sign you've done something
wrong. Only when you come to return from the function do you need a
single cast to ssize_t (provided you've checked for overflow first). I
strongly believe this entire commit should be reverted, and whatever
bug you were trying to fixed be fixed in another way without using the
wrong types and adding an array of unnecessary and/or dangerous casts.
Jess
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r367689 - head/lib/libutil
On 14 Nov 2020, at 19:04, Scott Long wrote:
> @@ -66,10 +67,16 @@ getlocalbase(char *path, size_t pathlen)
> #endif
>
> tmplen = strlcpy(path, tmppath, pathlen);
> - if ((tmplen < 0) || (tmplen >= (ssize_t)pathlen)) {
> + if ((tmplen < 0) || (tmplen >= pathlen)) {
I'd expect the LHS to give a compiler warning still.
Jess
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r367651 - head/usr.sbin/bhyve
On 14 Nov 2020, at 00:47, Ravi Pokala wrote:
>
>> +#define FIRMWARE_RELEASE_DATE "11/10/2020"
>
> Might I suggest "2020-11-10", as sorting, logic, ${DEITY}, and ISO-8601
> demand? ;-)
Alas that is a "feature" of the specification:
String number of the BIOS release date. The date string, if supplied,
is in either mm/dd/yy or mm/dd/ format. If the year portion of the
string is two digits, the year is assumed to be 19yy.
NOTE: The mm/dd/ format is required for SMBIOS version 2.3 and
later.
(SMBIOS Specification version 3.4.0c)
It might be worth putting a comment in so people don't accidentally
change it to the wrong thing in future though, given it's currently
ambiguous otherwise.
Jess
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r367648 - head/share/mk
On 13 Nov 2020, at 19:08, Ed Maste wrote:
>
> Author: emaste
> Date: Fri Nov 13 19:08:42 2020
> New Revision: 367648
> URL: https://svnweb.freebsd.org/changeset/base/367648
>
> Log:
> Fix `make makeman` after r367577
>
> WITH_INIT_ALL_ZERO and WITH_INIT_ALL_PATTERN are mutually exclusive.
> The .error when they were both set broke makeman so demote it to a
> warning (and presumably the compiler will fail on an error later on).
>
> We could improve this to make one take precedence but this is sufficient
> for now.
You won't get an error. Currently bsd.{prog,lib}.mk and kern.mk check
MK_INIT_ALL_ZERO then MK_INIT_ALL_PATTERN so the former takes
precedence, but I suspect that if you were to pass the compiler flags
for both the last flag would just override anything else, as is the
case for most compiler flags.
Jess
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r366976 - head/sys/kern
On 23 Oct 2020, at 16:56, Mateusz Guzik wrote:
>
> Author: mjg
> Date: Fri Oct 23 15:56:22 2020
> New Revision: 366976
> URL: https://svnweb.freebsd.org/changeset/base/366976
>
> Log:
> cache: reduce memory waste in struct namecache
>
> The previous scheme for calculating the total size was doing sizeof
> on the struct and then adding the wanted space for the buffer.
>
> nc_name is at offset 58 while sizeof(struct namecache) is 64.
> With CACHE_PATH_CUTOFF of 39 bytes and 1 byte of padding we were
> allocating 104 bytes for the entry and never accounting for the 6
> byte padding, wasting that space.
>
> Modified:
> head/sys/kern/vfs_cache.c
>
> Modified: head/sys/kern/vfs_cache.c
> ==
> --- head/sys/kern/vfs_cache.c Fri Oct 23 15:50:49 2020(r366975)
> +++ head/sys/kern/vfs_cache.c Fri Oct 23 15:56:22 2020(r366976)
> @@ -162,6 +162,7 @@ structnamecache_ts {
> struct timespec nc_time; /* timespec provided by fs */
> struct timespec nc_dotdottime; /* dotdot timespec provided by fs */
> int nc_ticks; /* ticks value when entry was added */
> + int nc_pad;
> struct namecache nc_nc;
> };
>
> @@ -172,12 +173,19 @@ struct namecache_ts {
> * alignment for everyone. Note this is a nop for 64-bit platforms.
> */
> #define CACHE_ZONE_ALIGNMENT UMA_ALIGNOF(time_t)
> -#define CACHE_PATH_CUTOFF 39
>
> -#define CACHE_ZONE_SMALL_SIZE(sizeof(struct namecache) +
> CACHE_PATH_CUTOFF + 1)
> -#define CACHE_ZONE_SMALL_TS_SIZE (sizeof(struct namecache_ts) +
> CACHE_PATH_CUTOFF + 1)
> -#define CACHE_ZONE_LARGE_SIZE(sizeof(struct namecache) +
> NAME_MAX + 1)
> -#define CACHE_ZONE_LARGE_TS_SIZE (sizeof(struct namecache_ts) + NAME_MAX
> + 1)
> +#ifdef __LP64__
> +#define CACHE_PATH_CUTOFF 45
> +#define CACHE_LARGE_PAD 6
> +#else
> +#define CACHE_PATH_CUTOFF 41
> +#define CACHE_LARGE_PAD 2
> +#endif
Is there any explanation of where these magic constants come from?
There should at least be a comment IMO, of better yet have a C
expression to evaluate them without needing #ifdef-based hard-coding
(which then annoys things like CHERI that has 128-bit pointers in its
pure capability kernel that causes us to have to go and
reverse-engineer where these numbers came from so we can figure out
what the right value should be for us).
Jess
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r366945 - head/share/man/man9
On 22 Oct 2020, at 19:00, Gleb Smirnoff wrote: > > Author: glebius > Date: Thu Oct 22 18:00:07 2020 > New Revision: 366945 > URL: https://svnweb.freebsd.org/changeset/base/366945 > > Log: > Fix typo > > Modified: > head/share/man/man9/pfil.9 > > Modified: head/share/man/man9/pfil.9 > == > --- head/share/man/man9/pfil.9Thu Oct 22 17:47:51 2020 > (r366944) > +++ head/share/man/man9/pfil.9Thu Oct 22 18:00:07 2020 > (r366945) > @@ -144,7 +144,7 @@ Link-layer packets. > .El > .Pp > Default rulesets are automatically linked to these heads to preserve > -historical behavavior. > +historical behaviour. This doesn't just fix the duplicated "av", it also changes the locale, which is not a typo. That may or may not be feature depending on who you are :) Jess ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r366833 - in head/sys: dev/iommu kern sys
On 19 Oct 2020, at 14:10, Ruslan Bukin wrote: > > +#ifndef _DEV_IOMMU_IOMMU_MSI_H_ > +#define _DEV_IOMMU_IOMMU_MSI_H_ > + > +#include > + > +struct iommu_unit; This seems unused, perhaps left from a previous patch version? Jess ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r366766 - head
On 16 Oct 2020, at 16:16, Kyle Evans wrote:
>
> Author: kevans
> Date: Fri Oct 16 15:16:23 2020
> New Revision: 366766
> URL: https://svnweb.freebsd.org/changeset/base/366766
>
> Log:
> Makefile: add a small blurb about building with gcc xtoolchain
>
> The key details are to install the appropriate flavor of devel/freebsd-gcc9
> and pass CROSS_TOOLCHAIN while building.
>
> Suggested by:kib
>
> Modified:
> head/Makefile
>
> Modified: head/Makefile
> ==
> --- head/Makefile Fri Oct 16 14:28:13 2020(r366765)
> +++ head/Makefile Fri Oct 16 15:16:23 2020(r366766)
> @@ -89,6 +89,15 @@
> # 10. `reboot'
> # 11. `make delete-old-libs' (in case no 3rd party program uses them anymore)
> #
> +# For individuals wanting to build from source with GCC from ports, first
> build
> +# or install an appropriate flavor of devel/freebsd-gcc9. The packages
> produced
> +# by this port are named "${TARGET_ARCH}-gcc9" -- note that not all
> +# architectures supported by FreeBSD have an external gcc toolchain
> available.
> +#
> +# Once the appropriate freebsd-gcc package is installed, simply pass
> +# CROSS_TOOLCHAIN=${TARGET_ARCH}-gcc9 while building with the above steps,
> +# e.g., `make buildworld CROSS_TOOLCHAIN=amd64-gcc9`.
Given GCC 10 is the latest GCC release so this is already a bit
outdated (though perhaps GCC 9 is still the recommended version for
FreeBSD now?), should this not avoid hard-coding a specific version and
thus s/9/N/ (or X)?
Jess
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r366736 - head/usr.sbin/kldxref
Author: jrtc27
Date: Thu Oct 15 18:03:14 2020
New Revision: 366736
URL: https://svnweb.freebsd.org/changeset/base/366736
Log:
kldxref: Avoid buffer overflows in parse_pnp_list
We convert a string like "W32:vendor/device" into "I:vendor;I:device",
where the output is longer than the input, but only allocate space equal
to the length of the input, leading to a buffer overflow.
Instead use open_memstream so we get a safe dynamically-grown buffer.
Found by: CHERI
Reviewed by: imp, jhb (mentor)
Approved by: imp, jhb (mentor)
Obtained from:CheriBSD
Differential Revision:https://reviews.freebsd.org/D26637
Modified:
head/usr.sbin/kldxref/kldxref.c
Modified: head/usr.sbin/kldxref/kldxref.c
==
--- head/usr.sbin/kldxref/kldxref.c Thu Oct 15 17:44:17 2020
(r366735)
+++ head/usr.sbin/kldxref/kldxref.c Thu Oct 15 18:03:14 2020
(r366736)
@@ -235,14 +235,17 @@ parse_pnp_list(const char *desc, char **new_desc, pnp_
const char *walker, *ep;
const char *colon, *semi;
struct pnp_elt *elt;
- char *nd;
char type[8], key[32];
int off;
+ size_t new_desc_size;
+ FILE *fp;
walker = desc;
ep = desc + strlen(desc);
off = 0;
- nd = *new_desc = malloc(strlen(desc) + 1);
+ fp = open_memstream(new_desc, _desc_size);
+ if (fp == NULL)
+ err(1, "Could not open new memory stream");
if (verbose > 1)
printf("Converting %s into a list\n", desc);
while (walker < ep) {
@@ -336,42 +339,44 @@ parse_pnp_list(const char *desc, char **new_desc, pnp_
off = elt->pe_offset + sizeof(void *);
}
if (elt->pe_kind & TYPE_PAIRED) {
- char *word, *ctx;
+ char *word, *ctx, newtype;
for (word = strtok_r(key, "/", );
word; word = strtok_r(NULL, "/", )) {
- sprintf(nd, "%c:%s;", elt->pe_kind &
TYPE_FLAGGED ? 'J' : 'I',
- word);
- nd += strlen(nd);
+ newtype = elt->pe_kind & TYPE_FLAGGED ? 'J' :
'I';
+ fprintf(fp, "%c:%s;", newtype, word);
}
-
}
else {
+ char newtype;
+
if (elt->pe_kind & TYPE_FLAGGED)
- *nd++ = 'J';
+ newtype = 'J';
else if (elt->pe_kind & TYPE_GE)
- *nd++ = 'G';
+ newtype = 'G';
else if (elt->pe_kind & TYPE_LE)
- *nd++ = 'L';
+ newtype = 'L';
else if (elt->pe_kind & TYPE_MASK)
- *nd++ = 'M';
+ newtype = 'M';
else if (elt->pe_kind & TYPE_INT)
- *nd++ = 'I';
+ newtype = 'I';
else if (elt->pe_kind == TYPE_D)
- *nd++ = 'D';
+ newtype = 'D';
else if (elt->pe_kind == TYPE_Z || elt->pe_kind ==
TYPE_E)
- *nd++ = 'Z';
+ newtype = 'Z';
else if (elt->pe_kind == TYPE_T)
- *nd++ = 'T';
+ newtype = 'T';
else
errx(1, "Impossible type %x\n", elt->pe_kind);
- *nd++ = ':';
- strcpy(nd, key);
- nd += strlen(nd);
- *nd++ = ';';
+ fprintf(fp, "%c:%s;", newtype, key);
}
}
- *nd++ = '\0';
+ if (ferror(fp) != 0) {
+ fclose(fp);
+ errx(1, "Exhausted space converting description %s", desc);
+ }
+ if (fclose(fp) != 0)
+ errx(1, "Failed to close memory stream");
return (0);
err:
errx(1, "Parse error of description string %s", desc);
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r366697 - head/usr.bin/xinstall
On 14 Oct 2020, at 14:28, Mateusz Guzik wrote:
>
> This should use copy_file_range (also available on Linux).
I assume this is a bootstrap tool and hence the system OS and version
is relevant. macOS does not have copy_file_range, and FreeBSD only has
it in -CURRENT so that would break building on 11.x and 12.x. So any
use would need to be guarded by preprocessor checks (and there are
still actively-supported Linux distributions out there that are based
on too-old versions of the kernel and/or glibc to include it).
(FYI macOS's equivalent is copyfile(3)... maybe one day it will adopt
the copy_file_range(2) interface too)
Jess
> On 10/14/20, Alex Richardson wrote:
>> Author: arichardson
>> Date: Wed Oct 14 12:28:41 2020
>> New Revision: 366697
>> URL: https://svnweb.freebsd.org/changeset/base/366697
>>
>> Log:
>> install(1): Avoid unncessary fstatfs() calls and use mmap() based on size
>>
>> According to git blame the trymmap() function was added in 1996 to skip
>> mmap() calls for NFS file systems. However, nowadays mmap() should be
>> perfectly safe even on NFS. Importantly, onl ufs and cd9660 file systems
>> were whitelisted so we don't use mmap() on ZFS. It also prevents the use
>> of mmap() when bootstrapping from macOS/Linux since on those systems the
>> trymmap() function was always returning zero due to the missing
>> MFSNAMELEN
>> define.
>>
>> This change keeps the trymmap() function but changes it to check whether
>> using mmap() can reduce the number of system calls that are required.
>> Using mmap() only reduces the number of system calls if we need multiple
>> read()
>> syscalls, i.e. if the file size is > MAXBSIZE. However, mmap() is more
>> expensive
>> than read() so this sets the threshold at 4 fewer syscalls. Additionally,
>> for
>> larger file size mmap() can significantly increase the number of page
>> faults,
>> so avoid it in that case.
>>
>> It's unclear whether using mmap() is ever faster than a read with an
>> appropriate
>> buffer size, but this change at least removes two unnecessary system
>> calls
>> for every file that is installed.
>>
>> Reviewed By:markj
>> Differential Revision: https://reviews.freebsd.org/D26041
>>
>> Modified:
>> head/usr.bin/xinstall/xinstall.c
>>
>> Modified: head/usr.bin/xinstall/xinstall.c
>> ==
>> --- head/usr.bin/xinstall/xinstall.c Wed Oct 14 10:12:39 2020
>> (r366696)
>> +++ head/usr.bin/xinstall/xinstall.c Wed Oct 14 12:28:41 2020
>> (r366697)
>> @@ -148,7 +148,7 @@ static void metadata_log(const char *, const char
>> *, s
>> const char *, const char *, off_t);
>> static int parseid(const char *, id_t *);
>> static int strip(const char *, int, const char *, char **);
>> -static int trymmap(int);
>> +static int trymmap(size_t);
>> static void usage(void);
>>
>> int
>> @@ -1087,7 +1087,7 @@ compare(int from_fd, const char *from_name __unused,
>> s
>> if (do_digest)
>> digest_init();
>> done_compare = 0;
>> -if (trymmap(from_fd) && trymmap(to_fd)) {
>> +if (trymmap(from_len) && trymmap(to_len)) {
>> p = mmap(NULL, from_len, PROT_READ, MAP_SHARED,
>> from_fd, (off_t)0);
>> if (p == MAP_FAILED)
>> @@ -1248,13 +1248,8 @@ copy(int from_fd, const char *from_name, int to_fd,
>> co
>>
>> digest_init();
>>
>> -/*
>> - * Mmap and write if less than 8M (the limit is so we don't totally
>> - * trash memory on big files. This is really a minor hack, but it
>> - * wins some CPU back.
>> - */
>> done_copy = 0;
>> -if (size <= 8 * 1048576 && trymmap(from_fd) &&
>> +if (trymmap((size_t)size) &&
>> (p = mmap(NULL, (size_t)size, PROT_READ, MAP_SHARED,
>> from_fd, (off_t)0)) != MAP_FAILED) {
>> nw = write(to_fd, p, size);
>> @@ -1523,20 +1518,23 @@ usage(void)
>> * return true (1) if mmap should be tried, false (0) if not.
>> */
>> static int
>> -trymmap(int fd)
>> +trymmap(size_t filesize)
>> {
>> -/*
>> - * The ifdef is for bootstrapping - f_fstypename doesn't exist in
>> - * pre-Lite2-merge systems.
>> - */
>> -#ifdef MFSNAMELEN
>> -struct statfs stfs;
>> -
>> -if (fstatfs(fd, ) != 0)
>> -return (0);
>> -if (strcmp(stfs.f_fstypename, "ufs") == 0 ||
>> -strcmp(stfs.f_fstypename, "cd9660") == 0)
>> -return (1);
>> -#endif
>> -return (0);
>> +/*
>> + * This function existed to skip mmap() for NFS file systems whereas
>> + * nowadays mmap() should be perfectly safe. Nevertheless, using mmap()
>> + * only reduces the number of system calls if we need multiple read()
>> + * syscalls, i.e. if the file size is > MAXBSIZE. However, mmap() is
>> + * more expensive than read() so set the threshold at 4 fewer syscalls.
>> +
Re: svn commit: r366634 - head/contrib/nvi/common
On 12 Oct 2020, at 11:42, Alex Richardson wrote: > --- head/contrib/nvi/common/common.h Mon Oct 12 10:42:19 2020 > (r366633) > +++ head/contrib/nvi/common/common.h Mon Oct 12 10:42:24 2020 > (r366634) > @@ -14,7 +14,7 @@ > #ifdef __linux__ > #include "/usr/include/db1/db.h" /* Only include db1. */ > #else > -#include "/usr/include/db.h" /* Only include db1. */ > +#include /* Only include db1. */ > #endif Can this not be expressed more nicely as the following? /* Only include db1 */ #if __has_include() #include #else #include #endif Jess ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r366487 - head/sys/riscv/riscv
Author: jrtc27
Date: Tue Oct 6 13:03:31 2020
New Revision: 366487
URL: https://svnweb.freebsd.org/changeset/base/366487
Log:
riscv: Remove outdated condition in page_fault_handler
Since r366355 and r366284 we panic on access faults rather than treating
them like page faults so this condition is never true.
Reviewed by: jhb (mentor), markj, mhorne
Approved by: jhb (mentor), markj, mhorne
Differential Revision:https://reviews.freebsd.org/D26686
Modified:
head/sys/riscv/riscv/trap.c
Modified: head/sys/riscv/riscv/trap.c
==
--- head/sys/riscv/riscv/trap.c Tue Oct 6 13:02:20 2020(r366486)
+++ head/sys/riscv/riscv/trap.c Tue Oct 6 13:03:31 2020(r366487)
@@ -220,8 +220,7 @@ page_fault_handler(struct trapframe *frame, int usermo
va = trunc_page(stval);
- if ((frame->tf_scause == EXCP_FAULT_STORE) ||
- (frame->tf_scause == EXCP_STORE_PAGE_FAULT)) {
+ if (frame->tf_scause == EXCP_STORE_PAGE_FAULT) {
ftype = VM_PROT_WRITE;
} else if (frame->tf_scause == EXCP_INST_PAGE_FAULT) {
ftype = VM_PROT_EXECUTE;
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r366486 - head/sys/riscv/riscv
Author: jrtc27 Date: Tue Oct 6 13:02:20 2020 New Revision: 366486 URL: https://svnweb.freebsd.org/changeset/base/366486 Log: riscv: Handle supervisor instruction page faults We should never take instruction page faults when in the kernel, but by using the standard page fault code we should get a more-informative message about faulting on a NOFAULT page rather than branching to the default case here and printing an "Unknown kernel exception ..." message. Reviewed by: jhb (mentor), markj Approved by: jhb (mentor), markj Differential Revision:https://reviews.freebsd.org/D26685 Modified: head/sys/riscv/riscv/trap.c Modified: head/sys/riscv/riscv/trap.c == --- head/sys/riscv/riscv/trap.c Tue Oct 6 12:57:54 2020(r366485) +++ head/sys/riscv/riscv/trap.c Tue Oct 6 13:02:20 2020(r366486) @@ -290,6 +290,7 @@ do_trap_supervisor(struct trapframe *frame) break; case EXCP_STORE_PAGE_FAULT: case EXCP_LOAD_PAGE_FAULT: + case EXCP_INST_PAGE_FAULT: page_fault_handler(frame, 0); break; case EXCP_BREAKPOINT: ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r366484 - head/sys/riscv/riscv
Author: jrtc27
Date: Tue Oct 6 12:56:29 2020
New Revision: 366484
URL: https://svnweb.freebsd.org/changeset/base/366484
Log:
riscv: De-Arm a few names
These names were inherited from the arm64 port and should be changed to
the RISC-V terminology.
Reviewed by: jhb (mentor), kp, markj
Approved by: jhb (mentor), kp, markj
Differential Revision:https://reviews.freebsd.org/D26671
Modified:
head/sys/riscv/riscv/exception.S
head/sys/riscv/riscv/trap.c
Modified: head/sys/riscv/riscv/exception.S
==
--- head/sys/riscv/riscv/exception.STue Oct 6 11:29:08 2020
(r366483)
+++ head/sys/riscv/riscv/exception.STue Oct 6 12:56:29 2020
(r366484)
@@ -40,12 +40,12 @@ __FBSDID("$FreeBSD$");
#include
#include
-.macro save_registers el
+.macro save_registers mode
addisp, sp, -(TF_SIZE)
sd ra, (TF_RA)(sp)
-.if \el == 0 /* We came from userspace. */
+.if \mode == 0 /* We came from userspace. */
sd gp, (TF_GP)(sp)
.option push
.option norelax
@@ -88,7 +88,7 @@ __FBSDID("$FreeBSD$");
sd a6, (TF_A + 6 * 8)(sp)
sd a7, (TF_A + 7 * 8)(sp)
-.if \el == 1
+.if \mode == 1
/* Store kernel sp */
li t1, TF_SIZE
add t0, sp, t1
@@ -110,9 +110,9 @@ __FBSDID("$FreeBSD$");
sd t0, (TF_SCAUSE)(sp)
.endm
-.macro load_registers el
+.macro load_registers mode
ld t0, (TF_SSTATUS)(sp)
-.if \el == 0
+.if \mode == 0
/* Ensure user interrupts will be enabled on eret */
li t1, SSTATUS_SPIE
or t0, t0, t1
@@ -130,7 +130,7 @@ __FBSDID("$FreeBSD$");
ld t0, (TF_SEPC)(sp)
csrwsepc, t0
-.if \el == 0
+.if \mode == 0
/* We go to userspace. Load user sp */
ld t0, (TF_SP)(sp)
csrwsscratch, t0
Modified: head/sys/riscv/riscv/trap.c
==
--- head/sys/riscv/riscv/trap.c Tue Oct 6 11:29:08 2020(r366483)
+++ head/sys/riscv/riscv/trap.c Tue Oct 6 12:56:29 2020(r366484)
@@ -158,7 +158,7 @@ dump_regs(struct trapframe *frame)
}
static void
-svc_handler(struct trapframe *frame)
+ecall_handler(struct trapframe *frame)
{
struct thread *td;
@@ -172,7 +172,7 @@ svc_handler(struct trapframe *frame)
}
static void
-data_abort(struct trapframe *frame, int usermode)
+page_fault_handler(struct trapframe *frame, int usermode)
{
struct vm_map *map;
uint64_t stval;
@@ -290,7 +290,7 @@ do_trap_supervisor(struct trapframe *frame)
break;
case EXCP_STORE_PAGE_FAULT:
case EXCP_LOAD_PAGE_FAULT:
- data_abort(frame, 0);
+ page_fault_handler(frame, 0);
break;
case EXCP_BREAKPOINT:
#ifdef KDTRACE_HOOKS
@@ -353,11 +353,11 @@ do_trap_user(struct trapframe *frame)
case EXCP_STORE_PAGE_FAULT:
case EXCP_LOAD_PAGE_FAULT:
case EXCP_INST_PAGE_FAULT:
- data_abort(frame, 1);
+ page_fault_handler(frame, 1);
break;
case EXCP_USER_ECALL:
frame->tf_sepc += 4;/* Next instruction */
- svc_handler(frame);
+ ecall_handler(frame);
break;
case EXCP_ILLEGAL_INSTRUCTION:
#ifdef FPE
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r366279 - head/sys/riscv/include
Author: jrtc27 Date: Wed Sep 30 02:21:38 2020 New Revision: 366279 URL: https://svnweb.freebsd.org/changeset/base/366279 Log: riscv: Define __PCI_REROUTE_INTERRUPT Every other architecture defines this and this is required for interrupts to work when using QEMU's PCI VirtIO devices (which all report an interrupt line of 0) for two reasons. Firstly, interrupt line 0 is wrong; they use one of 0x20-0x23 with the lines being cycled across devices like normal. Moreover, RISC-V uses INTRNG, whose IRQs are virtual as indices into its irq_map, so even if we have the right interrupt line we still need to try and route the interrupt in order to ultimately call into intr_map_irq and get back a unique index into the map for the given line, otherwise we will use whatever happens to be in irq_map[line] (which for QEMU where the line is initialised to 0 results in using the first allocated interrupt, namely the RTC on IRQ 11 at time of commit). Note that pci_assign_interrupt will still do the wrong thing for INTRNG when using a tunable, as it will bypass INTRNG entirely and use the tunable's value as the index into irq_map, when it should instead (indirectly) call intr_map_irq to allocate a new entry for the given IRQ and treat the tunable as stating the physical line in use, which is what one would expect. This, however, is a problem shared by all INTRNG architectures, and not exclusive to RISC-V. Reviewed by: kib Approved by: kib Differential Revision:https://reviews.freebsd.org/D26564 Modified: head/sys/riscv/include/param.h Modified: head/sys/riscv/include/param.h == --- head/sys/riscv/include/param.h Wed Sep 30 02:18:09 2020 (r366278) +++ head/sys/riscv/include/param.h Wed Sep 30 02:21:38 2020 (r366279) @@ -42,6 +42,8 @@ #defineSTACKALIGNBYTES (16 - 1) #defineSTACKALIGN(p) ((uint64_t)(p) & ~STACKALIGNBYTES) +#define __PCI_REROUTE_INTERRUPT + #ifndef MACHINE #defineMACHINE "riscv" #endif ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r366169 - head/sys/mips/include
On 26 Sep 2020, at 00:01, Alexander Richardson wrote:
>
>
> On Fri, 25 Sep 2020, 20:04 Justin Hibbits, wrote:
> Author: jhibbits
> Date: Fri Sep 25 19:04:03 2020
> New Revision: 366169
> URL: https://svnweb.freebsd.org/changeset/base/366169
>
> Log:
> mips: Fix compat32 library builds from r366162
>
> Re-add the a_ptr and a_fcn fields to Elf32_Auxinfo.
>
> MFC after:1 week
> Sponsored by: Juniper Networks, Inc.
>
> Modified:
> head/sys/mips/include/elf.h
>
> Modified: head/sys/mips/include/elf.h
> ==
> --- head/sys/mips/include/elf.h Fri Sep 25 19:02:49 2020(r366168)
> +++ head/sys/mips/include/elf.h Fri Sep 25 19:04:03 2020(r366169)
> @@ -105,6 +105,10 @@ typedef struct { /* Auxiliary vector entry on initial
> int a_type; /* Entry type. */
> union {
> int a_val; /* Integer value. */
> +#ifndef __mips_n64
> + void*a_ptr; /* Address. */
> + void(*a_fcn)(void); /* Function pointer (not used). */
> +#endif
> } a_un;
> } Elf32_Auxinfo;
>
> Not sure what the current minimal compiler versions are, but maybe this
> should be #if __SIZEOF_POINTER__ == 4 instead of checking the ABI? This would
> break CHERI-MIPS kernels since we don't define __mips_n64 for the
> pure-capability ABI (128-bit pointers). However, we don't really do compat 32
> right now so it probably doesn't matter much.
Or why not just #if defined(__mips_o32) || defined(__mips_n32)?
Jess
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r366064 - head/sbin/fsck_msdosfs
On 23 Sep 2020, at 07:52, Xin LI wrote:
>
> --- head/sbin/fsck_msdosfs/dir.c Wed Sep 23 04:09:02 2020
> (r366063)
> +++ head/sbin/fsck_msdosfs/dir.c Wed Sep 23 06:52:22 2020
> (r366064)
> @@ -388,7 +388,8 @@ static int
> checksize(struct fat_descriptor *fat, u_char *p, struct dosDirEntry *dir)
> {
> int ret = FSOK;
> - size_t physicalSize;
> + size_t chainsize;
> + u_int64_t physicalSize;
Is there a reason for using the non-standard u_int64_t rather than uint64_t?
Jess
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r365932 - head/sys/sys
Author: jrtc27 Date: Sun Sep 20 23:20:18 2020 New Revision: 365932 URL: https://svnweb.freebsd.org/changeset/base/365932 Log: atomic_common.h: Fix the volatile qualifier placement in atomic_load_ptr This was broken in r357940 which introduced the __typeof use. We need the volatile qualifier to be on the pointee not the pointer otherwise it does nothing. This was found by mhorne in D26498, noticing there was a problem (a spin loop condition was hoisted for RISC-V boot code) but not the root cause of it. Reported by: mhorne Reviewed by: mhorne, mjg Approved by: mhorne, mjg Differential Revision:https://reviews.freebsd.org/D26500 Modified: head/sys/sys/atomic_common.h Modified: head/sys/sys/atomic_common.h == --- head/sys/sys/atomic_common.hSun Sep 20 22:16:24 2020 (r365931) +++ head/sys/sys/atomic_common.hSun Sep 20 23:20:18 2020 (r365932) @@ -41,7 +41,7 @@ #defineatomic_load_short(p)(*(volatile u_short *)(p)) #defineatomic_load_int(p) (*(volatile u_int *)(p)) #defineatomic_load_long(p) (*(volatile u_long *)(p)) -#defineatomic_load_ptr(p) (*(volatile __typeof(p))(p)) +#defineatomic_load_ptr(p) (*(volatile __typeof(*p) *)(p)) #defineatomic_load_8(p)(*(volatile uint8_t *)(p)) #defineatomic_load_16(p) (*(volatile uint16_t *)(p)) #defineatomic_load_32(p) (*(volatile uint32_t *)(p)) ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r365836 - head/share/mk
> On 17 Sep 2020, at 18:23, Jessica Clarke wrote: > >> On 17 Sep 2020, at 18:05, Rodney W. Grimes wrote: >> >>> On Thu, Sep 17, 2020 at 9:39 AM Steffen Nurpmeso wrote: >>> >>>> Alex Richardson wrote in >>>> <202009171507.08hf7qns080...@repo.freebsd.org>: >>>> |Author: arichardson >>>> |Date: Thu Sep 17 15:07:25 2020 >>>> |New Revision: 365836 >>>> |URL: https://svnweb.freebsd.org/changeset/base/365836 >>>> | >>>> |Log: >>>> | Stop using lorder and ranlib when building libraries >>>> | >>>> | Use of ranlib or lorder is no longer necessary with current linkers >>>> | (probably anything newer than ~1990) and ar's ability to create an >>>> object >>>> | index and symbol table in the archive. >>>> | Currently the build system uses lorder+tsort to sort the .o files in >>>> | dependency order so that a single-pass linker can use them. However, >>>> | we can use the -s flag to ar to add an index to the .a file which makes >>>> | lorder unnecessary. >>>> | Running ar -s is equivalent to running ranlib afterwards, so we can >>>> also >>>> | skip the ranlib invocation. >>>> >>>> That ranlib thing yes (for long indeed), but i have vague memories >>>> that the tsort/lorder ordering was also meant to keep the things >>>> which heavily interdepend nearby each other. (Luckily Linux >>>> always had at least tsort available.) >>>> This no longer matters for all the platforms FreeBSD supports? >>>> >>> >>> tsort has no notion of how dependent the modules are, just an order that >>> allows a single pass through the .a file (otherwise you'd need to list the >>> .a file multiple times on the command line absent ranlib). That's the >>> original purpose of tsort. tsort, lsort, and ranlib all arrived in 7th >>> edition unix on a PDP-11, where size was more important than proximity to >>> locations (modulo overlays, which this doesn't affect at all). >>> >>> There were some issues of long vs short jumps on earlier architectures that >>> this helped (since you could only jump 16MB, for example). However, there >>> were workarounds for this issue on those platforms too. And if you have a >>> program that this does make a difference, then you can still use >>> tsort/lorder. They are still in the system. >>> >>> I doubt you could measure a difference here today. I doubt, honestly, that >>> anybody will notice at all. >> >> The x86 archicture has relative jmps of differning lengths, even in long mode >> there is support for rel8 and rel32. > > That's irrelevant though for several reasons: > > 1. The compiler has already decided on what jump instructions to use based on > the requested code model (unless you're on RISC-V and using GNU bfd ld as > that supports linker relaxations that actually delete instruction bytes). > > 2. The linker is still free to reorder input sections however it likes, it > doesn't have to follow the order of the input files (and the files within > any archive). Hm actually that's only true for archives; it needs to respect the order of files on the command line for things like crti.o to work. But regardless, the other points (and this one, partially) still hold. > 3. If you care about those kinds of optimisations you should use link-time > optimisation which will likely do far more useful things than just optimise > branches, but again isn't constrained by the order of the input files, it > can lay out the code exactly how it wants. > > Not to mention that this is just a topological sort, not a clustering sort. > > Jess ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r365836 - head/share/mk
> On 17 Sep 2020, at 18:05, Rodney W. Grimes wrote: > >> On Thu, Sep 17, 2020 at 9:39 AM Steffen Nurpmeso wrote: >> >>> Alex Richardson wrote in >>> <202009171507.08hf7qns080...@repo.freebsd.org>: >>> |Author: arichardson >>> |Date: Thu Sep 17 15:07:25 2020 >>> |New Revision: 365836 >>> |URL: https://svnweb.freebsd.org/changeset/base/365836 >>> | >>> |Log: >>> | Stop using lorder and ranlib when building libraries >>> | >>> | Use of ranlib or lorder is no longer necessary with current linkers >>> | (probably anything newer than ~1990) and ar's ability to create an >>> object >>> | index and symbol table in the archive. >>> | Currently the build system uses lorder+tsort to sort the .o files in >>> | dependency order so that a single-pass linker can use them. However, >>> | we can use the -s flag to ar to add an index to the .a file which makes >>> | lorder unnecessary. >>> | Running ar -s is equivalent to running ranlib afterwards, so we can >>> also >>> | skip the ranlib invocation. >>> >>> That ranlib thing yes (for long indeed), but i have vague memories >>> that the tsort/lorder ordering was also meant to keep the things >>> which heavily interdepend nearby each other. (Luckily Linux >>> always had at least tsort available.) >>> This no longer matters for all the platforms FreeBSD supports? >>> >> >> tsort has no notion of how dependent the modules are, just an order that >> allows a single pass through the .a file (otherwise you'd need to list the >> .a file multiple times on the command line absent ranlib). That's the >> original purpose of tsort. tsort, lsort, and ranlib all arrived in 7th >> edition unix on a PDP-11, where size was more important than proximity to >> locations (modulo overlays, which this doesn't affect at all). >> >> There were some issues of long vs short jumps on earlier architectures that >> this helped (since you could only jump 16MB, for example). However, there >> were workarounds for this issue on those platforms too. And if you have a >> program that this does make a difference, then you can still use >> tsort/lorder. They are still in the system. >> >> I doubt you could measure a difference here today. I doubt, honestly, that >> anybody will notice at all. > > The x86 archicture has relative jmps of differning lengths, even in long mode > there is support for rel8 and rel32. That's irrelevant though for several reasons: 1. The compiler has already decided on what jump instructions to use based on the requested code model (unless you're on RISC-V and using GNU bfd ld as that supports linker relaxations that actually delete instruction bytes). 2. The linker is still free to reorder input sections however it likes, it doesn't have to follow the order of the input files (and the files within any archive). 3. If you care about those kinds of optimisations you should use link-time optimisation which will likely do far more useful things than just optimise branches, but again isn't constrained by the order of the input files, it can lay out the code exactly how it wants. Not to mention that this is just a topological sort, not a clustering sort. Jess ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r364637 - head/sys/kern
On 6 Sep 2020, at 21:46, Alan Somers wrote: > > On Mon, Aug 24, 2020 at 3:01 AM Mateusz Guzik wrote: > Author: mjg > Date: Mon Aug 24 09:00:57 2020 > New Revision: 364637 > URL: https://svnweb.freebsd.org/changeset/base/364637 > > Log: > cache: lockless reverse lookup > > This enables fully scalable operation for getcwd and significantly improves > realpath. > > For example: > PATH_CUSTOM=/usr/src ./getcwd_processes -t 104 > before: 1550851 > after: 380135380 > > Tested by:pho > > Modified: > head/sys/kern/vfs_cache.c > > Modified: head/sys/kern/vfs_cache.c > == > --- head/sys/kern/vfs_cache.c Mon Aug 24 09:00:07 2020(r364636) > +++ head/sys/kern/vfs_cache.c Mon Aug 24 09:00:57 2020(r364637) > @@ -477,6 +485,8 @@ STATNODE_COUNTER(shrinking_skipped, > static void cache_zap_locked(struct namecache *ncp); > static int vn_fullpath_hardlink(struct nameidata *ndp, char **retbuf, > char **freebuf, size_t *buflen); > +static int vn_fullpath_any_smr(struct vnode *vp, struct vnode *rdir, char > *buf, > +char **retbuf, size_t *buflen, bool slash_prefixed, size_t addend); > static int vn_fullpath_any(struct vnode *vp, struct vnode *rdir, char *buf, > char **retbuf, size_t *buflen); > static int vn_fullpath_dir(struct vnode *vp, struct vnode *rdir, char *buf, > @@ -2476,9 +2486,17 @@ vn_getcwd(char *buf, char **retbuf, size_t *buflen) > > What does the "smr" stand for? Safe Memory Reclamation (see sys/sys/smr.h and sys/kern/subr_smr.c). Jess ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r364799 - in head: share/man/man9 sys/crypto/ccp sys/dev/cxgbe/crypto sys/dev/sec sys/kern sys/opencrypto
On 26 Aug 2020, at 20:21, Brandon Bergren wrote: > On Wed, Aug 26, 2020, at 2:19 PM, Jessica Clarke wrote: >> On 26 Aug 2020, at 20:16, Brandon Bergren wrote: >>> On Tue, Aug 25, 2020, at 9:37 PM, Alan Somers wrote: >>>> Author: asomers >>>> Date: Wed Aug 26 02:37:42 2020 >>>> New Revision: 364799 >>>> URL: https://svnweb.freebsd.org/changeset/base/364799 >>>> >>>> Modified: head/sys/dev/sec/sec.c >>>> == >>>> --- head/sys/dev/sec/sec.c Wed Aug 26 02:13:27 2020(r364798) >>>> +++ head/sys/dev/sec/sec.c Wed Aug 26 02:37:42 2020(r364799) >>>> @@ -851,6 +851,9 @@ sec_desc_map_dma(struct sec_softc *sc, struct sec_dma_ >>>>case CRYPTO_BUF_MBUF: >>>>size = m_length(crp->crp_buf.cb_mbuf, NULL); >>>>break; >>>> + case CRYPTO_BUF_VMPAGE: >>>> + size = PAGE_SIZE - cb->cb_vm_page_offset; >>>> + break; >>>>default: >>>>return (EINVAL); >>>>} >>> >>> Uh, where is cb coming from? Shouldn't this be using >>> crp->crp_buf.cb_vm_page_offset? This is causing a build failure on powerpc >>> and powerpcspe. I don't see why other platforms aren't also erroring out >>> here. >> >> Because it's PowerPC-specific: >> >> sys/conf/files.powerpc:dev/sec/sec.c optionalsec mpc85xx >> >> Jess >> >> > > No, I mean literally. What scope is cb coming from? It's not a variable that > is in scope for that function as far as I can tell. Oh no I agree it's wrong and your proposal sounds correct. I was just explaining why it's only noticed in PowerPC builds. Jess ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r364799 - in head: share/man/man9 sys/crypto/ccp sys/dev/cxgbe/crypto sys/dev/sec sys/kern sys/opencrypto
On 26 Aug 2020, at 20:16, Brandon Bergren wrote: > On Tue, Aug 25, 2020, at 9:37 PM, Alan Somers wrote: >> Author: asomers >> Date: Wed Aug 26 02:37:42 2020 >> New Revision: 364799 >> URL: https://svnweb.freebsd.org/changeset/base/364799 >> >> Modified: head/sys/dev/sec/sec.c >> == >> --- head/sys/dev/sec/sec.c Wed Aug 26 02:13:27 2020(r364798) >> +++ head/sys/dev/sec/sec.c Wed Aug 26 02:37:42 2020(r364799) >> @@ -851,6 +851,9 @@ sec_desc_map_dma(struct sec_softc *sc, struct sec_dma_ >> case CRYPTO_BUF_MBUF: >> size = m_length(crp->crp_buf.cb_mbuf, NULL); >> break; >> +case CRYPTO_BUF_VMPAGE: >> +size = PAGE_SIZE - cb->cb_vm_page_offset; >> +break; >> default: >> return (EINVAL); >> } > > Uh, where is cb coming from? Shouldn't this be using > crp->crp_buf.cb_vm_page_offset? This is causing a build failure on powerpc > and powerpcspe. I don't see why other platforms aren't also erroring out here. Because it's PowerPC-specific: sys/conf/files.powerpc:dev/sec/sec.c optionalsec mpc85xx Jess ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r364822 - in head/crypto/openssl/crypto: aes/asm bn/asm chacha/asm ec/asm modes/asm poly1305/asm sha/asm
On 26 Aug 2020, at 19:33, Dimitry Andric wrote: > > On 26 Aug 2020, at 19:13, Ian Lepore wrote: >> >> On Wed, 2020-08-26 at 19:04 +0200, Mateusz Guzik wrote: >>> On 8/26/20, Jung-uk Kim wrote: Author: jkim Date: Wed Aug 26 16:55:28 2020 New Revision: 364822 URL: https://svnweb.freebsd.org/changeset/base/364822 Log: Fix Clang version detection. We prepend "FreeBSD" to Clang version string. This broke compiler test for AVX instruction support. >>> >>> What about other software checking in similar fashion? imo the right >>> fix is to stop mucking with the way clang reports itself >>> >> >> Maybe it would be better to not modify the start of the string. >> Instead of >> >> FreeBSD clang version 9.0.1 (g...@github.com:llvm/llvm-project.git >> c1a0a213378a458fbea1a5c77b315c7dce08fd05) (based on LLVM 9.0.1) >> >> maybe >> >> clang version 9.0.1 for FreeBSD (g...@github.com:llvm/llvm-project.git >> c1a0a213378a458fbea1a5c77b315c7dce08fd05) (based on LLVM 9.0.1) > > We have been doing this since, well, forever. And this way actually > originates from upstream, we only define the CLANG_VENDOR macro. I see > no reason to change this after all those years. > > A better question is, why these perl scripts "suddenly" started failing? > Or have they also failed since forever, and it was only noticed now? Ah, digging deeper it gets more interesting. All those scripts check for "based on LLVM X.Y", a suffix printed for vendor builds. However, that was dropped in https://reviews.llvm.org/D69925 as it's redundant, thereby breaking this detection. So it's fallout from LLVM 10. Also the scripts aren't failing in a sense, they just don't know what compiler is in use so they fall back on not enabling AVX. Jess ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r364822 - in head/crypto/openssl/crypto: aes/asm bn/asm chacha/asm ec/asm modes/asm poly1305/asm sha/asm
On 26 Aug 2020, at 18:04, Mateusz Guzik wrote: > > On 8/26/20, Jung-uk Kim wrote: >> Author: jkim >> Date: Wed Aug 26 16:55:28 2020 >> New Revision: 364822 >> URL: https://svnweb.freebsd.org/changeset/base/364822 >> >> Log: >> Fix Clang version detection. >> >> We prepend "FreeBSD" to Clang version string. This broke compiler test >> for >> AVX instruction support. >> > > What about other software checking in similar fashion? imo the right > fix is to stop mucking with the way clang reports itself Apple's LLVM also does the same thing. Whilst it may be better to leave the string alone, it is sometimes useful information and, given the existence of Apple's LLVM, well-behaved software should already be dealing with this properly; based on this commit, upstream OpenSSL seems like it suffers the same bug on macOS. Jess ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r364190 - head/tools/build
On 13 Aug 2020, at 17:22, Rodney W. Grimes wrote:
>
>> Author: arichardson
>> Date: Thu Aug 13 14:14:46 2020
>> New Revision: 364190
>> URL: https://svnweb.freebsd.org/changeset/base/364190
>>
>> Log:
>> Add pwd to the list of tools that are linked to $WORLDTMP/legacy
>
> Since "sh" is already in this list, and our "sh" has a builtin pwd
> that does the correct thing with pwd -P this should not be needed.
>
> Or are we contininue to use the host "sh" for far too long?
>
> For me from ancient days of hand bootstrapping BSD sources onto
> another system sh(1) and make(1) are the first 2 tools to get
> working.
The issue is that r364174 used `env pwd -P` rather than just `pwd -P`. With
that fixed, this should be revertible; even if the bootstrap sh isn't being
used at this point, I don't know of any contemporary sh-compatible shell that
doesn't implement pwd as a builtin (but surely we are using the bootstrap sh by
this point otherwise BUILD_WITH_STRICT_TMPPATH would have complained about sh).
Jess
>> After r364166 and r364174, crunchgen needs a pwd binary in $PATH instead
>> of using a hardcoded absolute path. This commit is needed for
>> BUILD_WITH_STRICT_TMPPATH builds (currently not on by default).
>>
>> Modified:
>> head/tools/build/Makefile
>>
>> Modified: head/tools/build/Makefile
>> ==
>> --- head/tools/build/MakefileThu Aug 13 13:59:31 2020
>> (r364189)
>> +++ head/tools/build/MakefileThu Aug 13 14:14:46 2020
>> (r364190)
>> @@ -113,8 +113,8 @@ SYSINCS+=${SRCTOP}/sys/sys/font.h
>> # Linux/MacOS since we only use flags that are supported by all of them.
>> _host_tools_to_symlink= basename bzip2 bunzip2 chmod chown cmp comm cp
>> date dd \
>> dirname echo env false find fmt gzip gunzip head hostname id ln ls \
>> -mkdir mv nice patch rm realpath sh sleep stat tee touch tr true uname \
>> -uniq wc which
>> +mkdir mv nice patch pwd rm realpath sh sleep stat tee touch tr true \
>> +uname uniq wc which
>>
>> # We also need a symlink to the absolute path to the make binary used for
>> # the toplevel makefile. This is not necessarily the same as `which make`
>>
>
> --
> Rod Grimes rgri...@freebsd.org
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r364166 - head/usr.sbin/crunch/crunchgen
On 13 Aug 2020, at 16:33, Ian Lepore wrote: > On Wed, 2020-08-12 at 09:24 -0700, Rodney W. Grimes wrote: >>> On 12 Aug 2020, at 17:10, Rodney W. Grimes < >>> free...@gndrsh.dnsmgr.net> wrote: > Author: arichardson > Date: Wed Aug 12 15:49:06 2020 > New Revision: 364166 > URL: https://svnweb.freebsd.org/changeset/base/364166 > > Log: > Fix crunchgen usage of mkstemp() > > On Glibc systems mkstemp can only be used once with the same > template > string since it will be modified in-place and no longer > contain any 'X' chars. > It is fine to reuse the same file here but we need to be > explicit and use > open() instead of mkstemp() on the second use. > > While touching this file also avoid a hardcoded /bin/pwd since > that may not > work when building on non-FreeBSD systems. This may cause some grief, as now pwd may use a shell builtin and often shell builtin's return a cwd that is not a true full path, ie it may contain symlink compontents in the path. /bin/sh: # cd /tmp/b # /bin/pwd /tmp/a # pwd /tmp/b # ls -lag /tmp/? lrwxr-xr-x 1 root wheel 1 Aug 12 16:06 /tmp/b -> a /tmp/a: total 17 drwxr-xr-x 2 root wheel2 Aug 12 16:06 . drwxrwxrwt 18 root wheel 248 Aug 12 16:06 .. >>> >>> There's the question of whether that really matters; both values >>> are in >>> some sense correct. But if you want to restore the old behaviour, I >>> believe `env pwd` is the portable way to do so? >> >> You have cut the context, but the code has a comment that >> states it is doing this to remove symbolic links, so this >> change infact undoes something that was being done intentionally. >> >> I do believe also that a "env pwd" would do the right thing >> as well. >> > > Or just use "pwd -P" and avoid invoking multiple programs when the > shell can do all the work. Indeed, my suggestion was solving the wrong problem. r364174 added the -P but also needlessly added env too; -P is part of POSIX so any conforming pwd will support it, builtin or not. Jess ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r364166 - head/usr.sbin/crunch/crunchgen
On 12 Aug 2020, at 17:10, Rodney W. Grimes wrote: > > [ Charset UTF-8 unsupported, converting... ] >> Author: arichardson >> Date: Wed Aug 12 15:49:06 2020 >> New Revision: 364166 >> URL: https://svnweb.freebsd.org/changeset/base/364166 >> >> Log: >> Fix crunchgen usage of mkstemp() >> >> On Glibc systems mkstemp can only be used once with the same template >> string since it will be modified in-place and no longer contain any 'X' >> chars. >> It is fine to reuse the same file here but we need to be explicit and use >> open() instead of mkstemp() on the second use. >> >> While touching this file also avoid a hardcoded /bin/pwd since that may not >> work when building on non-FreeBSD systems. > > This may cause some grief, as now pwd may use a shell builtin > and often shell builtin's return a cwd that is not a true > full path, ie it may contain symlink compontents in the > path. > > /bin/sh: > > # cd /tmp/b > # /bin/pwd > /tmp/a > # pwd > /tmp/b > # ls -lag /tmp/? > lrwxr-xr-x 1 root wheel 1 Aug 12 16:06 /tmp/b -> a > > /tmp/a: > total 17 > drwxr-xr-x 2 root wheel2 Aug 12 16:06 . > drwxrwxrwt 18 root wheel 248 Aug 12 16:06 .. There's the question of whether that really matters; both values are in some sense correct. But if you want to restore the old behaviour, I believe `env pwd` is the portable way to do so? Jess ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r364124 - head/cddl/contrib/opensolaris/lib/libdtrace/common
On 11 Aug 2020, at 17:46, Alex Richardson wrote: > > Author: arichardson > Date: Tue Aug 11 16:46:54 2020 > New Revision: 364124 > URL: https://svnweb.freebsd.org/changeset/base/364124 > > Log: > Fix libdtrace build with zsh as /bin/sh > > When zsh runs in POSIX sh mode it does not support the -e flag to echo. > Use printf instead of echo to avoid the "-e" characters being printed. > > Obtained from: CheriBSD > Reviewed By: markj > Differential Revision: https://reviews.freebsd.org/D26026 > > Modified: > head/cddl/contrib/opensolaris/lib/libdtrace/common/mkerrno.sh > head/cddl/contrib/opensolaris/lib/libdtrace/common/mkerrtags.sh > head/cddl/contrib/opensolaris/lib/libdtrace/common/mknames.sh > head/cddl/contrib/opensolaris/lib/libdtrace/common/mksignal.sh > > Modified: head/cddl/contrib/opensolaris/lib/libdtrace/common/mkerrno.sh > == > --- head/cddl/contrib/opensolaris/lib/libdtrace/common/mkerrno.sh Tue Aug > 11 16:46:48 2020(r364123) > +++ head/cddl/contrib/opensolaris/lib/libdtrace/common/mkerrno.sh Tue Aug > 11 16:46:54 2020(r364124) > @@ -24,16 +24,15 @@ > # Copyright 2003 Sun Microsystems, Inc. All rights reserved. > # Use is subject to license terms. > # > -#ident "%Z%%M% %I% %E% SMI" > set -e > > -echo "\ > -/*\n\ > - * Copyright 2003 Sun Microsystems, Inc. All rights reserved.\n\ > - * Use is subject to license terms.\n\ > - */\n\ > -\n\ > -#pragma ident\t\"%Z%%M%\t%I%\t%E% SMI\"\n" > +printf "%s" " > +/* > + * Copyright 2003 Sun Microsystems, Inc. All rights reserved. > + * Use is subject to license terms. > + */ > + > +" Surely these would be much better as a heredoc? cat
Re: svn commit: r363091 - in head/contrib/bc: . include manuals src tests tests/bc
On 30 Jul 2020, at 17:40, Ravi Pokala wrote: > > -Original Message- > From: on behalf of Jessica Clarke > > Date: 2020-07-30, Thursday at 09:35 > To: Baptiste Daroussin > Cc: Stefan Eßer , src-committers > , , > > Subject: Re: svn commit: r363091 - in head/contrib/bc: . include manuals src > tests tests/bc > >On 30 Jul 2020, at 17:31, Baptiste Daroussin wrote: >> On Thu, Jul 30, 2020 at 05:28:19PM +0100, Jessica Clarke wrote: >>> On 30 Jul 2020, at 17:20, Baptiste Daroussin wrote: >>>> On Sat, Jul 11, 2020 at 07:33:19AM +, Stefan Eßer wrote: >>>>> Author: se >>>>> Date: Sat Jul 11 07:33:18 2020 >>>>> New Revision: 363091 >>>>> URL: https://svnweb.freebsd.org/changeset/base/363091 >>>>> >>>>> Log: >>>>> Update to version 3.1.3 >>>>> >>>> Jumping on that commit, since the switch from our previous bc. >>>> >>>> The output of the interactive bc has changed, the previous version had a >>>> clean >>>> UI, the new version "pollutes" the output with plenty of lines about the >>>> copyright: >>>> >>>> >>>> Copyright (c) 2018-2020 Gavin D. Howard and contributors >>>> Report bugs at: https://git.yzena.com/gavin/bc >>>> >>>> This is free software with ABSOLUTELY NO WARRANTY. >>>> >>>> >>>> Imagine if all programs where doing that, it would be painful, do you think >>>> upstream can be convinced to remove those lines? >>>> >>>> I no the GNU version also has the same polluted output which was one of the >>>> reason I was happy with out previous version of bc. >>> >>> By default both will print such a banner if and only if being called >>> interactively. You can disable the banner explicitly with -q/--quiet >>> for both GNU bc and this bc. I agree it's a bit noisy and would be >>> nicer to not have that printed, but it's not without precedent for >>> REPL-like things. >> >> Yes it is not without precedent for REPL-like things, still I dislike this >> and >> would be happy to get bc interactive be as nice as the previous one we had :) >> >> If not I will deal with it and just yell internally each time I run it :D > >`alias bc='bc -q'` / `alias bc bc -q` and preserve your inner zen? :) > >Jess > > I was actually about to complain about the new `dc' not exiting after > evaluating a '-e' expression, without an explicit 'q'. But then I noticed the > "DC_EXPR_EXIT" envvar, which restores the desired behavior. That lead me to > discover "DC_ENV_ARGS" and, correspondingly, "BC_ENV_ARGS"; that last one > would be helpful here. That does feel like the wrong default; even GNU dc doesn't do that, and the principle of least surprise would suggest exiting is the right thing to do. It's also unlikely you want to evaluate something and then use it interactively. Jess ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r363091 - in head/contrib/bc: . include manuals src tests tests/bc
On 30 Jul 2020, at 17:31, Baptiste Daroussin wrote: > On Thu, Jul 30, 2020 at 05:28:19PM +0100, Jessica Clarke wrote: >> On 30 Jul 2020, at 17:20, Baptiste Daroussin wrote: >>> On Sat, Jul 11, 2020 at 07:33:19AM +, Stefan Eßer wrote: >>>> Author: se >>>> Date: Sat Jul 11 07:33:18 2020 >>>> New Revision: 363091 >>>> URL: https://svnweb.freebsd.org/changeset/base/363091 >>>> >>>> Log: >>>> Update to version 3.1.3 >>>> >>> Jumping on that commit, since the switch from our previous bc. >>> >>> The output of the interactive bc has changed, the previous version had a >>> clean >>> UI, the new version "pollutes" the output with plenty of lines about the >>> copyright: >>> >>> >>> Copyright (c) 2018-2020 Gavin D. Howard and contributors >>> Report bugs at: https://git.yzena.com/gavin/bc >>> >>> This is free software with ABSOLUTELY NO WARRANTY. >>> >>> >>> Imagine if all programs where doing that, it would be painful, do you think >>> upstream can be convinced to remove those lines? >>> >>> I no the GNU version also has the same polluted output which was one of the >>> reason I was happy with out previous version of bc. >> >> By default both will print such a banner if and only if being called >> interactively. You can disable the banner explicitly with -q/--quiet >> for both GNU bc and this bc. I agree it's a bit noisy and would be >> nicer to not have that printed, but it's not without precedent for >> REPL-like things. > > Yes it is not without precedent for REPL-like things, still I dislike this and > would be happy to get bc interactive be as nice as the previous one we had :) > > If not I will deal with it and just yell internally each time I run it :D `alias bc='bc -q'` / `alias bc bc -q` and preserve your inner zen? :) Jess ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r363091 - in head/contrib/bc: . include manuals src tests tests/bc
On 30 Jul 2020, at 17:20, Baptiste Daroussin wrote: > On Sat, Jul 11, 2020 at 07:33:19AM +, Stefan Eßer wrote: >> Author: se >> Date: Sat Jul 11 07:33:18 2020 >> New Revision: 363091 >> URL: https://svnweb.freebsd.org/changeset/base/363091 >> >> Log: >> Update to version 3.1.3 >> > Jumping on that commit, since the switch from our previous bc. > > The output of the interactive bc has changed, the previous version had a clean > UI, the new version "pollutes" the output with plenty of lines about the > copyright: > > > Copyright (c) 2018-2020 Gavin D. Howard and contributors > Report bugs at: https://git.yzena.com/gavin/bc > > This is free software with ABSOLUTELY NO WARRANTY. > > > Imagine if all programs where doing that, it would be painful, do you think > upstream can be convinced to remove those lines? > > I no the GNU version also has the same polluted output which was one of the > reason I was happy with out previous version of bc. By default both will print such a banner if and only if being called interactively. You can disable the banner explicitly with -q/--quiet for both GNU bc and this bc. I agree it's a bit noisy and would be nicer to not have that printed, but it's not without precedent for REPL-like things. Jess ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r363574 - in head/sys: conf riscv/conf riscv/riscv
Author: jrtc27
Date: Sun Jul 26 18:21:02 2020
New Revision: 363574
URL: https://svnweb.freebsd.org/changeset/base/363574
Log:
riscv: Include syscon_power device driver in GENERIC kernel config
QEMU's RISC-V virt machine provides syscon-power and syscon-reset
devices as the means by which to shutdown and reboot. We also need to
ensure that we have attached the syscon_generic device before attaching
any syscon_power devices, and so we introduce a new riscv_syscon device
akin to aw_syscon added in r327936. Currently the SiFive test finisher
is used as the specific implementation of such a syscon device.
Reviewed by: br, brooks (mentor), jhb (mentor)
Approved by: br, brooks (mentor), jhb (mentor)
Obtained from:CheriBSD
Differential Revision:https://reviews.freebsd.org/D25725
Added:
head/sys/riscv/riscv/riscv_syscon.c (contents, props changed)
Modified:
head/sys/conf/files.riscv
head/sys/riscv/conf/GENERIC
Modified: head/sys/conf/files.riscv
==
--- head/sys/conf/files.riscv Sun Jul 26 18:19:50 2020(r363573)
+++ head/sys/conf/files.riscv Sun Jul 26 18:21:02 2020(r363574)
@@ -57,6 +57,7 @@ riscv/riscv/ofw_machdep.c optionalfdt
riscv/riscv/plic.c standard
riscv/riscv/pmap.c standard
riscv/riscv/riscv_console.coptionalrcons
+riscv/riscv/riscv_syscon.c optionalext_resources syscon
riscv_syscon fdt
riscv/riscv/sbi.c standard
riscv/riscv/soc.c standard
riscv/riscv/stack_machdep.coptionalddb | stack
Modified: head/sys/riscv/conf/GENERIC
==
--- head/sys/riscv/conf/GENERIC Sun Jul 26 18:19:50 2020(r363573)
+++ head/sys/riscv/conf/GENERIC Sun Jul 26 18:21:02 2020(r363574)
@@ -77,6 +77,13 @@ options INTRNG
# RISC-V SBI console
device rcons
+# EXT_RESOURCES pseudo devices
+optionsEXT_RESOURCES
+device clk
+device syscon
+device syscon_power
+device riscv_syscon
+
# Bus drivers
device pci
Added: head/sys/riscv/riscv/riscv_syscon.c
==
--- /dev/null 00:00:00 1970 (empty, because file is newly added)
+++ head/sys/riscv/riscv/riscv_syscon.c Sun Jul 26 18:21:02 2020
(r363574)
@@ -0,0 +1,84 @@
+/*-
+ * SPDX-License-Identifier: BSD-2-Clause-FreeBSD
+ *
+ * Copyright (c) 2018 Kyle Evans
+ * Copyright (c) 2020 Jessica Clarke
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *notice, this list of conditions and the following disclaimer in the
+ *documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+/*
+ * RISC-V syscon driver. Used as a generic interface by QEMU's virt machine for
+ * describing the SiFive test finisher as a power and reset controller.
+ */
+
+#include
+__FBSDID("$FreeBSD$");
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+
+#include
+#include
+
+static struct ofw_compat_data compat_data[] = {
+ {"sifive,test0",1},
+ {"sifive,test1",1},
+ {NULL, 0}
+};
+
+static int
+riscv_syscon_probe(device_t dev)
+{
+
+ if (!ofw_bus_status_okay(dev))
+ return (ENXIO);
+ if (ofw_bus_search_compatible(dev, compat_data)->ocd_data == 0)
+ return (ENXIO);
+
+ device_set_desc(dev, "RISC-V syscon");
+ return (BUS_PROBE_DEFAULT);
+}
+
+static device_method_t riscv_syscon_methods[] = {
+ DEVMETHOD(device_probe, riscv_syscon_probe),
+
+ DEVMETHOD_END
+};
+
+DEFINE_CLASS_1(riscv_syscon, riscv_sysco
svn commit: r363573 - in head/sys: conf dev/extres/syscon
Author: jrtc27
Date: Sun Jul 26 18:19:50 2020
New Revision: 363573
URL: https://svnweb.freebsd.org/changeset/base/363573
Log:
Add syscon power and reset control device driver
This device driver supports both syscon-power and syscon-reset devices,
as specified in [1] and [2]. These provide a very simple interface for
power and reset control, and among other things are used by QEMU's virt
machine on RISC-V. A separate commit will enable this on RISC-V, as that
requires adding a RISC-V-specific riscv_syscon akin to r327936's
aw_syscon.
[1]
https://www.kernel.org/doc/Documentation/devicetree/bindings/power/reset/syscon-poweroff.txt
[2]
https://www.kernel.org/doc/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
Reviewed by: brooks (mentor), jhb (mentor)
Approved by: brooks (mentor), jhb (mentor)
Obtained from:CheriBSD
Differential Revision:https://reviews.freebsd.org/D25724
Added:
head/sys/dev/extres/syscon/syscon_power.c (contents, props changed)
Modified:
head/sys/conf/files
Modified: head/sys/conf/files
==
--- head/sys/conf/files Sun Jul 26 18:17:36 2020(r363572)
+++ head/sys/conf/files Sun Jul 26 18:19:50 2020(r363573)
@@ -1702,6 +1702,7 @@ dev/extres/regulator/regulator_fixed.coptional ext_re
dev/extres/syscon/syscon.c optional ext_resources syscon
dev/extres/syscon/syscon_generic.c optional ext_resources syscon fdt
dev/extres/syscon/syscon_if.m optional ext_resources syscon
+dev/extres/syscon/syscon_power.c optional ext_resources syscon
syscon_power fdt
dev/fb/fbd.c optional fbd | vt
dev/fb/fb_if.m standard
dev/fb/splash.coptional sc splash
Added: head/sys/dev/extres/syscon/syscon_power.c
==
--- /dev/null 00:00:00 1970 (empty, because file is newly added)
+++ head/sys/dev/extres/syscon/syscon_power.c Sun Jul 26 18:19:50 2020
(r363573)
@@ -0,0 +1,198 @@
+/*-
+ * SPDX-License-Identifier: BSD-2-Clause-FreeBSD
+ *
+ * Copyright (c) 2020 Jessica Clarke
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *notice, this list of conditions and the following disclaimer in the
+ *documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+/*
+ * Driver for simple syscon poweroff and reset devices. The device tree
+ * specifications are fully described at:
+ *
+ *
https://www.kernel.org/doc/Documentation/devicetree/bindings/power/reset/syscon-poweroff.txt
+ *
https://www.kernel.org/doc/Documentation/devicetree/bindings/power/reset/syscon-reboot.txt
+ */
+
+#include
+__FBSDID("$FreeBSD$");
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+#include
+#include
+#include
+
+#include "syscon_if.h"
+#include "syscon.h"
+
+struct syscon_power_softc {
+ struct syscon *regmap;
+ uint32_toffset;
+ uint32_tvalue;
+ uint32_tmask;
+ boolreboot;
+ eventhandler_tagshutdown_tag;
+};
+
+static void
+syscon_power_shutdown_final(device_t dev, int howto)
+{
+ struct syscon_power_softc *sc;
+ bool write;
+
+ sc = device_get_softc(dev);
+ if (sc->reboot)
+ write = (howto & RB_HALT) == 0;
+ else
+ write = (howto & RB_POWEROFF) != 0;
+
+ if (write)
+ SYSCON_MODIFY_4(sc->regmap, sc->offset, sc->mask,
+ sc->value & sc->mask);
+}
+
+static int
+syscon_power_probe(device_t dev)
+{
+
+ if (!ofw_bus_status_okay(dev))
+ return (ENXIO
svn commit: r363572 - in head/stand/efi/loader/arch: arm riscv
Author: jrtc27
Date: Sun Jul 26 18:17:36 2020
New Revision: 363572
URL: https://svnweb.freebsd.org/changeset/base/363572
Log:
loader: Avoid -Wpointer-to-int cast warnings for Arm and RISC-V
On RISC-V, Clang warns with:
cast to smaller integer type 'unsigned int' from 'void (*)(void *)'
Instead, use %p as the standard format specifier for printing pointers.
Whilst Arm's pointer size is the same as unsigned, it's still cleaner to
use the right thing there too.
Reviewed by: brooks (mentor), emaste
Approved by: brooks (mentor), emaste
Differential Revision:https://reviews.freebsd.org/D25718
Modified:
head/stand/efi/loader/arch/arm/exec.c
head/stand/efi/loader/arch/riscv/exec.c
Modified: head/stand/efi/loader/arch/arm/exec.c
==
--- head/stand/efi/loader/arch/arm/exec.c Sun Jul 26 18:15:16 2020
(r363571)
+++ head/stand/efi/loader/arch/arm/exec.c Sun Jul 26 18:17:36 2020
(r363572)
@@ -77,7 +77,7 @@ __elfN(arm_exec)(struct preloaded_file *fp)
entry = efi_translate(e->e_entry);
- printf("Kernel entry at 0x%x...\n", (unsigned)entry);
+ printf("Kernel entry at %p...\n", entry);
printf("Kernel args: %s\n", fp->f_args);
if ((error = bi_load(fp->f_args, , )) != 0) {
Modified: head/stand/efi/loader/arch/riscv/exec.c
==
--- head/stand/efi/loader/arch/riscv/exec.c Sun Jul 26 18:15:16 2020
(r363571)
+++ head/stand/efi/loader/arch/riscv/exec.c Sun Jul 26 18:17:36 2020
(r363572)
@@ -63,7 +63,7 @@ __elfN(exec)(struct preloaded_file *fp)
entry = efi_translate(e->e_entry);
- printf("Kernel entry at 0x%x...\n", (unsigned)entry);
+ printf("Kernel entry at %p...\n", entry);
printf("Kernel args: %s\n", fp->f_args);
if ((error = bi_load(fp->f_args, , )) != 0) {
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r363571 - in head/sys: conf dev/goldfish riscv/conf
Author: jrtc27
Date: Sun Jul 26 18:15:16 2020
New Revision: 363571
URL: https://svnweb.freebsd.org/changeset/base/363571
Log:
Add Goldfish RTC device driver for RISC-V
This device was originally used as part of the goldfish virtual hardware
platform used for emulating Android on QEMU, but is now also used as the
RTC for the RISC-V virt machine in QEMU. It provides a simple 64-bit
nanosecond timer exposed via a pair of memory-mapped 32-bit registers,
although only with 1s granularity.
Reviewed by: brooks (mentor), jhb (mentor), kp
Approved by: brooks (mentor), jhb (mentor), kp
Obtained from:CheriBSD
Differential Revision:https://reviews.freebsd.org/D25717
Added:
head/sys/dev/goldfish/
head/sys/dev/goldfish/goldfish_rtc.c (contents, props changed)
Modified:
head/sys/conf/files
head/sys/riscv/conf/GENERIC
Modified: head/sys/conf/files
==
--- head/sys/conf/files Sun Jul 26 18:12:54 2020(r363570)
+++ head/sys/conf/files Sun Jul 26 18:15:16 2020(r363571)
@@ -1736,6 +1736,7 @@ dev/fxp/if_fxp.c optional fxp
dev/fxp/inphy.coptional fxp
dev/gem/if_gem.c optional gem
dev/gem/if_gem_pci.c optional gem pci
+dev/goldfish/goldfish_rtc.coptional goldfish_rtc fdt
dev/gpio/dwgpio/dwgpio.c optional gpio dwgpio fdt
dev/gpio/dwgpio/dwgpio_bus.c optional gpio dwgpio fdt
dev/gpio/dwgpio/dwgpio_if.moptional gpio dwgpio fdt
Added: head/sys/dev/goldfish/goldfish_rtc.c
==
--- /dev/null 00:00:00 1970 (empty, because file is newly added)
+++ head/sys/dev/goldfish/goldfish_rtc.cSun Jul 26 18:15:16 2020
(r363571)
@@ -0,0 +1,182 @@
+/*-
+ * SPDX-License-Identifier: BSD-2-Clause-FreeBSD
+ *
+ * Copyright (c) 2020 Jessica Clarke
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *notice, this list of conditions and the following disclaimer in the
+ *documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+/*
+ * RTC for the goldfish virtual hardware platform implemented in QEMU,
+ * initially for Android but now also used for RISC-V's virt machine.
+ *
+ *
https://android.googlesource.com/platform/external/qemu/+/master/docs/GOLDFISH-VIRTUAL-HARDWARE.TXT
+ */
+
+#include
+__FBSDID("$FreeBSD$");
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+
+#include
+#include
+#include
+
+#include "clock_if.h"
+
+#defineGOLDFISH_RTC_TIME_LOW 0x00
+#defineGOLDFISH_RTC_TIME_HIGH 0x04
+
+struct goldfish_rtc_softc {
+ struct resource *res;
+ int rid;
+ struct mtx mtx;
+};
+
+static int
+goldfish_rtc_probe(device_t dev)
+{
+
+ if (!ofw_bus_status_okay(dev))
+ return (ENXIO);
+
+ if (ofw_bus_is_compatible(dev, "google,goldfish-rtc")) {
+ device_set_desc(dev, "Goldfish RTC");
+ return (BUS_PROBE_DEFAULT);
+ }
+
+ return (ENXIO);
+}
+
+static int
+goldfish_rtc_attach(device_t dev)
+{
+ struct goldfish_rtc_softc *sc;
+
+ sc = device_get_softc(dev);
+
+ sc->rid = 0;
+ sc->res = bus_alloc_resource_any(dev, SYS_RES_MEMORY, >rid,
+ RF_ACTIVE);
+ if (sc->res == NULL) {
+ device_printf(dev, "could not allocate resource\n");
+ return (ENXIO);
+ }
+
+ mtx_init(>mtx, device_get_nameunit(dev), NULL, MTX_DEF);
+
+ /*
+* Register as a system realtime clock with 1 second resolution.
+*/
+ clock_register_flags(dev, 100, CLOCKF_SETTIME_NO_ADJ);
+
Re: svn commit: r362285 - head/sys/dev/pci
On 18 Jun 2020, at 13:23, Ed Maste wrote:
> On Wed, 17 Jun 2020 at 15:56, Andrew Turner wrote:
>>
>> Author: andrew
>> Date: Wed Jun 17 19:56:17 2020
>> New Revision: 362285
>> URL: https://svnweb.freebsd.org/changeset/base/362285
>>
>> Log:
>> Clean up the pci host generic driver
> ...
>>
>> + /* Translate the address from a PCI address to a physical address */
>> + switch (type) {
>> + case SYS_RES_IOPORT:
>> + case SYS_RES_MEMORY:
>> + found = false;
>> + for (i = 0; i < MAX_RANGES_TUPLES; i++) {
>> + pci_base = sc->ranges[i].pci_base;
>> + phys_base = sc->ranges[i].phys_base;
>> + size = sc->ranges[i].size;
>> +
>> + if (start < pci_base || start >= pci_base + size)
>> + continue;
>
> Should the second condition be end instead? markj had this comment on
> the old version in review D20884.
The code previously had:
> if ((rman_get_start(r) >= pci_base) && (rman_get_start(r) < (pci_base +
> size)))
> found = 1;
> break;
> }
The new code is just the inverted form of that.
Jess
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r362186 - head/sys/dev/virtio/network
Author: jrtc27
Date: Sun Jun 14 22:39:34 2020
New Revision: 362186
URL: https://svnweb.freebsd.org/changeset/base/362186
Log:
vtnet: Fix regression introduced in r361944
For legacy devices that don't support MrgRxBuf (such as bhyve pre-r358180),
r361944 failed to update the receive handler to account for the additional
padding introduced by the unused num_buffers field that is now always present
in struct vtnet_rx_header. Thus, calculate the padding dynamically based on
vtnet_hdr_size.
PR: 247242
Reported by: thj
Tested by:thj
Modified:
head/sys/dev/virtio/network/if_vtnet.c
Modified: head/sys/dev/virtio/network/if_vtnet.c
==
--- head/sys/dev/virtio/network/if_vtnet.c Sun Jun 14 21:07:12 2020
(r362185)
+++ head/sys/dev/virtio/network/if_vtnet.c Sun Jun 14 22:39:34 2020
(r362186)
@@ -1819,9 +1819,10 @@ vtnet_rxq_eof(struct vtnet_rxq *rxq)
adjsz = sizeof(struct vtnet_rx_header);
/*
* Account for our pad inserted between the header
-* and the actual start of the frame.
+* and the actual start of the frame. This includes
+* the unused num_buffers when using a legacy device.
*/
- len += VTNET_RX_HEADER_PAD;
+ len += adjsz - sc->vtnet_hdr_size;
} else {
mhdr = mtod(m, struct virtio_net_hdr_mrg_rxbuf *);
nbufs = mhdr->num_buffers;
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r361944 - in head/sys/dev/virtio: . network
On 14 Jun 2020, at 22:22, Tom Jones wrote: > On Sun, Jun 14, 2020 at 09:56:03PM +0100, Jessica Clarke wrote: >> On 14 Jun 2020, at 20:51, Tom Jones wrote: >>> On Mon, Jun 08, 2020 at 09:51:36PM +0000, Jessica Clarke wrote: >>>> Author: jrtc27 >>>> Date: Mon Jun 8 21:51:36 2020 >>>> New Revision: 361944 >>>> URL: https://svnweb.freebsd.org/changeset/base/361944 >>>> >>>> Log: >>>> virtio: Support non-legacy network device and queue >>>> >>>> The non-legacy interface always defines num_buffers in the header, >>>> regardless of whether VIRTIO_NET_F_MRG_RXBUF, just leaving it unused. We >>>> also need to ensure our virtqueue doesn't filter out VIRTIO_F_VERSION_1 >>>> during negotiation, as it supports non-legacy transports just fine. This >>>> fixes network packet transmission on TinyEMU. >>>> >>>> Reviewed by: br, brooks (mentor), jhb (mentor) >>>> Approved by: br, brooks (mentor), jhb (mentor) >>>> Differential Revision: https://reviews.freebsd.org/D25132 >>>> >>>> Modified: >>>> head/sys/dev/virtio/network/if_vtnet.c >>>> head/sys/dev/virtio/network/if_vtnetvar.h >>>> head/sys/dev/virtio/virtio.c >>>> head/sys/dev/virtio/virtqueue.c >>>> >>> >>> Hi Jessica, >>> >>> After updating my current bhyve vm today (on a 12.1 host), networking no >>> longer >>> works. Reverting this commit seems to resolve the issue. I think vtnet is >>> not >>> passing enough data up to the ip layer. >>> >>> If I capture on the tap interface for the vm I see arp requests and arp >>> replies, however kern.msgbuf is full of: >>> >>> <5>arp: short packet received on vtnet0 >>> >>> and netstat does not see any replies to arp requests: >>> >>> root@freebsd-current:~ # netstat -s -p arp >>> arp: >>> 11 ARP requests sent >>> 0 ARP requests failed to sent >>> 0 ARP replies sent >>> 0 ARP requests received >>> 0 ARP replies received >>> 0 ARP packets received >>> 24 total packets dropped due to no ARP entry >>> 2 ARP entrys timed out >>> 0 Duplicate IPs seen >>> >>> If I set up an arp entry manually I can see ICMP echo requests and >>> responses on >>> the tap interface, but the vm does not see the responses. >>> >>> root@freebsd-current:~ # netstat -s -p ip >>> ip: >>> 7 total packets received >>> 0 bad header checksums >>> 0 with size smaller than minimum >>> 7 with data size < data length >>> 0 with ip length > max ip packet size >>> 0 with header length < data size >>> 0 with data length < header length >>> >>> The line >>> >>> 7 with data size < data length >>> >>> makes me think that vtnet is truncating packets. >>> >>> markj pointed me at this bug in irc which might also be related: >>> >>> https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=247242 >> >> Hi Tom, >> Sorry about that; it seems bhyve hits the "legacy and no MrgRxBuf" >> case. Could you please try the patch below? >> >> Jess >> > > This changed fixed the issue for me. Please feel free to add > > Tested By: thj > > when you commit. Great, thanks for the report. > In testing I this lor went by, I wonder if this is something you care about: > > acquiring duplicate lock of same type: "vtnet0-rx0" > 1st vtnet0-rx0 @ > /usr/home/tj/code/freebsd/projects/review-D25220/sys/dev/virtio/network/if_vtnet.c:1780 > 2nd vtnet0-rx0 @ > /usr/home/tj/code/freebsd/projects/review-D25220/sys/kern/subr_taskqueue.c:281 > stack backtrace: > #0 0x80c32881 at witness_debugger+0x71 > #1 0x80ba3e54 at __mtx_lock_flags+0x94 > #2 0x80c24bd2 at taskqueue_enqueue+0x42 > #3 0x80a1af99 at vtnet_rxq_tq_intr+0xb9 > #4 0x80c2520a at taskqueue_run_locked+0xaa > #5 0x80c26284 at taskqueue_thread_loop+0x94 > #6 0x80b830e0 at fork_exit+0x80 > #7 0x81040eae at fork_trampoline+0xe Hm, I think that's just a false-positive, because if_vtnet constructs the taskqueue using the same name as its own internal mutexes. Though the locking around vtnet_rx_vq_intr and vtnet_rxq_tq_intr is a bit fishy given they're rather similar yet inconsistent. I would imagine rxq->vtnrx_stats.vrxs_rescheduled is supposed to be protected by that mutex, but wouldn't like to say whether taskqueue_enqueue needs to be. Vincenzo, you recently touched code around there, perhaps you could be persuaded to have a quick look?.. Jess ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r361944 - in head/sys/dev/virtio: . network
On 14 Jun 2020, at 20:51, Tom Jones wrote:
> On Mon, Jun 08, 2020 at 09:51:36PM +0000, Jessica Clarke wrote:
>> Author: jrtc27
>> Date: Mon Jun 8 21:51:36 2020
>> New Revision: 361944
>> URL: https://svnweb.freebsd.org/changeset/base/361944
>>
>> Log:
>> virtio: Support non-legacy network device and queue
>>
>> The non-legacy interface always defines num_buffers in the header,
>> regardless of whether VIRTIO_NET_F_MRG_RXBUF, just leaving it unused. We
>> also need to ensure our virtqueue doesn't filter out VIRTIO_F_VERSION_1
>> during negotiation, as it supports non-legacy transports just fine. This
>> fixes network packet transmission on TinyEMU.
>>
>> Reviewed by:br, brooks (mentor), jhb (mentor)
>> Approved by:br, brooks (mentor), jhb (mentor)
>> Differential Revision: https://reviews.freebsd.org/D25132
>>
>> Modified:
>> head/sys/dev/virtio/network/if_vtnet.c
>> head/sys/dev/virtio/network/if_vtnetvar.h
>> head/sys/dev/virtio/virtio.c
>> head/sys/dev/virtio/virtqueue.c
>>
>
> Hi Jessica,
>
> After updating my current bhyve vm today (on a 12.1 host), networking no
> longer
> works. Reverting this commit seems to resolve the issue. I think vtnet is not
> passing enough data up to the ip layer.
>
> If I capture on the tap interface for the vm I see arp requests and arp
> replies, however kern.msgbuf is full of:
>
> <5>arp: short packet received on vtnet0
>
> and netstat does not see any replies to arp requests:
>
> root@freebsd-current:~ # netstat -s -p arp
> arp:
>11 ARP requests sent
>0 ARP requests failed to sent
>0 ARP replies sent
>0 ARP requests received
>0 ARP replies received
>0 ARP packets received
>24 total packets dropped due to no ARP entry
>2 ARP entrys timed out
>0 Duplicate IPs seen
>
> If I set up an arp entry manually I can see ICMP echo requests and responses
> on
> the tap interface, but the vm does not see the responses.
>
> root@freebsd-current:~ # netstat -s -p ip
> ip:
>7 total packets received
>0 bad header checksums
>0 with size smaller than minimum
>7 with data size < data length
>0 with ip length > max ip packet size
>0 with header length < data size
>0 with data length < header length
>
> The line
>
>7 with data size < data length
>
> makes me think that vtnet is truncating packets.
>
> markj pointed me at this bug in irc which might also be related:
>
> https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=247242
Hi Tom,
Sorry about that; it seems bhyve hits the "legacy and no MrgRxBuf"
case. Could you please try the patch below?
Jess
diff --git a/sys/dev/virtio/network/if_vtnet.c
b/sys/dev/virtio/network/if_vtnet.c
index 7a0859cc0eb1..7e10b75f7f66 100644
--- a/sys/dev/virtio/network/if_vtnet.c
+++ b/sys/dev/virtio/network/if_vtnet.c
@@ -1819,9 +1819,10 @@ vtnet_rxq_eof(struct vtnet_rxq *rxq)
adjsz = sizeof(struct vtnet_rx_header);
/*
* Account for our pad inserted between the header
- * and the actual start of the frame.
+ * and the actual start of the frame. This includes
+ * the unused num_buffers when using a legacy device.
*/
-len += VTNET_RX_HEADER_PAD;
+len += adjsz - sc->vtnet_hdr_size;
} else {
mhdr = mtod(m, struct virtio_net_hdr_mrg_rxbuf *);
nbufs = mhdr->num_buffers;
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r361944 - in head/sys/dev/virtio: . network
Author: jrtc27
Date: Mon Jun 8 21:51:36 2020
New Revision: 361944
URL: https://svnweb.freebsd.org/changeset/base/361944
Log:
virtio: Support non-legacy network device and queue
The non-legacy interface always defines num_buffers in the header,
regardless of whether VIRTIO_NET_F_MRG_RXBUF, just leaving it unused. We
also need to ensure our virtqueue doesn't filter out VIRTIO_F_VERSION_1
during negotiation, as it supports non-legacy transports just fine. This
fixes network packet transmission on TinyEMU.
Reviewed by: br, brooks (mentor), jhb (mentor)
Approved by: br, brooks (mentor), jhb (mentor)
Differential Revision:https://reviews.freebsd.org/D25132
Modified:
head/sys/dev/virtio/network/if_vtnet.c
head/sys/dev/virtio/network/if_vtnetvar.h
head/sys/dev/virtio/virtio.c
head/sys/dev/virtio/virtqueue.c
Modified: head/sys/dev/virtio/network/if_vtnet.c
==
--- head/sys/dev/virtio/network/if_vtnet.c Mon Jun 8 21:49:42 2020
(r361943)
+++ head/sys/dev/virtio/network/if_vtnet.c Mon Jun 8 21:51:36 2020
(r361944)
@@ -640,10 +640,13 @@ vtnet_setup_features(struct vtnet_softc *sc)
sc->vtnet_flags |= VTNET_FLAG_MAC;
}
- if (virtio_with_feature(dev, VIRTIO_NET_F_MRG_RXBUF)) {
+ if (virtio_with_feature(dev, VIRTIO_NET_F_MRG_RXBUF))
sc->vtnet_flags |= VTNET_FLAG_MRG_RXBUFS;
+
+ if (virtio_with_feature(dev, VIRTIO_NET_F_MRG_RXBUF) ||
+ virtio_with_feature(dev, VIRTIO_F_VERSION_1))
sc->vtnet_hdr_size = sizeof(struct virtio_net_hdr_mrg_rxbuf);
- } else
+ else
sc->vtnet_hdr_size = sizeof(struct virtio_net_hdr);
if (sc->vtnet_flags & VTNET_FLAG_MRG_RXBUFS)
@@ -1459,9 +1462,10 @@ vtnet_rxq_enqueue_buf(struct vtnet_rxq *rxq, struct mb
sglist_reset(sg);
if ((sc->vtnet_flags & VTNET_FLAG_MRG_RXBUFS) == 0) {
- MPASS(sc->vtnet_hdr_size == sizeof(struct virtio_net_hdr));
+ MPASS(sc->vtnet_hdr_size == sizeof(rxhdr->vrh_uhdr.hdr) ||
+ sc->vtnet_hdr_size == sizeof(rxhdr->vrh_uhdr.mhdr));
rxhdr = (struct vtnet_rx_header *) mdata;
- sglist_append(sg, >vrh_hdr, sc->vtnet_hdr_size);
+ sglist_append(sg, >vrh_uhdr, sc->vtnet_hdr_size);
offset = sizeof(struct vtnet_rx_header);
} else
offset = 0;
Modified: head/sys/dev/virtio/network/if_vtnetvar.h
==
--- head/sys/dev/virtio/network/if_vtnetvar.h Mon Jun 8 21:49:42 2020
(r361943)
+++ head/sys/dev/virtio/network/if_vtnetvar.h Mon Jun 8 21:51:36 2020
(r361944)
@@ -219,15 +219,20 @@ struct vtnet_softc {
* When mergeable buffers are not negotiated, the vtnet_rx_header structure
* below is placed at the beginning of the mbuf data. Use 4 bytes of pad to
* both keep the VirtIO header and the data non-contiguous and to keep the
- * frame's payload 4 byte aligned.
+ * frame's payload 4 byte aligned. Note that non-legacy drivers still want
+ * room for a full mergeable buffer header.
*
* When mergeable buffers are negotiated, the host puts the VirtIO header in
* the beginning of the first mbuf's data.
*/
#define VTNET_RX_HEADER_PAD4
struct vtnet_rx_header {
- struct virtio_net_hdr vrh_hdr;
- charvrh_pad[VTNET_RX_HEADER_PAD];
+ union {
+ struct virtio_net_hdr hdr;
+ struct virtio_net_hdr_mrg_rxbuf mhdr;
+ } vrh_uhdr;
+
+ charvrh_pad[VTNET_RX_HEADER_PAD];
} __packed;
/*
@@ -296,7 +301,8 @@ CTASSERT(sizeof(struct vtnet_mac_filter) <= PAGE_SIZE)
VIRTIO_NET_F_MRG_RXBUF| \
VIRTIO_NET_F_MQ | \
VIRTIO_RING_F_EVENT_IDX | \
- VIRTIO_RING_F_INDIRECT_DESC)
+ VIRTIO_RING_F_INDIRECT_DESC | \
+ VIRTIO_F_VERSION_1)
/*
* The VIRTIO_NET_F_HOST_TSO[46] features permit us to send the host
Modified: head/sys/dev/virtio/virtio.c
==
--- head/sys/dev/virtio/virtio.cMon Jun 8 21:49:42 2020
(r361943)
+++ head/sys/dev/virtio/virtio.cMon Jun 8 21:51:36 2020
(r361944)
@@ -79,6 +79,7 @@ static struct virtio_feature_desc virtio_common_featur
{ VIRTIO_RING_F_INDIRECT_DESC, "RingIndirect" },
{ VIRTIO_RING_F_EVENT_IDX, "EventIdx" },
{ VIRTIO_F_BAD_FEATURE, "BadFeature"},
+ { VIRTIO_F_VERSION_1, "Version1" },
{ 0, NULL }
};
Modified: head/sys/dev/virtio/virtqueue.c
==
--- head/sys/dev/virtio/virtqueue.c Mon Jun 8 21:49:42 2020
(r361943)
+++
svn commit: r361943 - head/sys/dev/virtio/mmio
Author: jrtc27 Date: Mon Jun 8 21:49:42 2020 New Revision: 361943 URL: https://svnweb.freebsd.org/changeset/base/361943 Log: virtio_mmio: Negotiate the upper half of the feature bits too The feature bits are exposed as a 32-bit register with 2 banks, so we should negotiate both halves. Notably, VIRTIO_F_VERSION_1 is in the upper half, and will be used in an upcoming commit. The PCI bus driver also has this bug, but the legacy BAR layout did not include selector registers and is rather different from the modern layout, so it remains solely as legacy. Reviewed by: br, brooks (mentor), jhb (mentor) Approved by: br, brooks (mentor), jhb (mentor) Differential Revision:https://reviews.freebsd.org/D25131 Modified: head/sys/dev/virtio/mmio/virtio_mmio.c Modified: head/sys/dev/virtio/mmio/virtio_mmio.c == --- head/sys/dev/virtio/mmio/virtio_mmio.c Mon Jun 8 21:38:52 2020 (r361942) +++ head/sys/dev/virtio/mmio/virtio_mmio.c Mon Jun 8 21:49:42 2020 (r361943) @@ -390,7 +390,13 @@ vtmmio_negotiate_features(device_t dev, uint64_t child sc = device_get_softc(dev); + vtmmio_write_config_4(sc, VIRTIO_MMIO_HOST_FEATURES_SEL, 1); host_features = vtmmio_read_config_4(sc, VIRTIO_MMIO_HOST_FEATURES); + host_features <<= 32; + + vtmmio_write_config_4(sc, VIRTIO_MMIO_HOST_FEATURES_SEL, 0); + host_features |= vtmmio_read_config_4(sc, VIRTIO_MMIO_HOST_FEATURES); + vtmmio_describe_features(sc, "host", host_features); /* @@ -402,6 +408,11 @@ vtmmio_negotiate_features(device_t dev, uint64_t child sc->vtmmio_features = features; vtmmio_describe_features(sc, "negotiated", features); + + vtmmio_write_config_4(sc, VIRTIO_MMIO_HOST_FEATURES_SEL, 1); + vtmmio_write_config_4(sc, VIRTIO_MMIO_GUEST_FEATURES, features >> 32); + + vtmmio_write_config_4(sc, VIRTIO_MMIO_HOST_FEATURES_SEL, 0); vtmmio_write_config_4(sc, VIRTIO_MMIO_GUEST_FEATURES, features); return (features); ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r361932 - head/sys/riscv/riscv
Author: jrtc27
Date: Mon Jun 8 17:57:21 2020
New Revision: 361932
URL: https://svnweb.freebsd.org/changeset/base/361932
Log:
riscv: Use SBI shutdown call to implement RB_POWEROFF
Currently we only call sbi_shutdown in cpu_reset, which means we reach
"Please press any key to reboot." even when RB_POWEROFF is set, and only
once the user presses a key do we then shutdown. Instead, register a
shutdown_final event handler and make an SBI shutdown call if
RB_POWEROFF is set.
Reviewed by: br, jhb (mentor), kp
Approved by: br, jhb (mentor), kp
Differential Revision:https://reviews.freebsd.org/D25183
Modified:
head/sys/riscv/riscv/sbi.c
Modified: head/sys/riscv/riscv/sbi.c
==
--- head/sys/riscv/riscv/sbi.c Mon Jun 8 17:40:39 2020(r361931)
+++ head/sys/riscv/riscv/sbi.c Mon Jun 8 17:57:21 2020(r361932)
@@ -29,8 +29,11 @@
__FBSDID("$FreeBSD$");
#include
+#include
#include
#include
+#include
+#include
#include
#include
@@ -80,6 +83,13 @@ sbi_get_mimpid(void)
return (SBI_CALL0(SBI_EXT_ID_BASE, SBI_BASE_GET_MIMPID));
}
+static void
+sbi_shutdown_final(void *dummy __unused, int howto)
+{
+ if ((howto & RB_POWEROFF) != 0)
+ sbi_shutdown();
+}
+
void
sbi_print_version(void)
{
@@ -187,3 +197,12 @@ sbi_init(void)
KASSERT(sbi_probe_extension(SBI_SHUTDOWN) != 0,
("SBI doesn't implement sbi_shutdown()"));
}
+
+static void
+sbi_late_init(void *dummy __unused)
+{
+ EVENTHANDLER_REGISTER(shutdown_final, sbi_shutdown_final, NULL,
+ SHUTDOWN_PRI_LAST);
+}
+
+SYSINIT(sbi, SI_SUB_KLD, SI_ORDER_ANY, sbi_late_init, NULL);
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r361010 - head/sys/riscv/riscv
Author: jrtc27
Date: Wed May 13 17:20:51 2020
New Revision: 361010
URL: https://svnweb.freebsd.org/changeset/base/361010
Log:
riscv: Fix pmap_protect for superpages
When protecting a superpage, we would previously fall through to the
non-superpage case and read the contents of the superpage as PTEs,
potentially modifying them and trying to look up underlying VM pages that
don't exist if they happen to look like PTEs we would care about. This led
to nginx causing an unexpected page fault in pmap_protect that panic'ed the
kernel. Instead, if we see a superpage, we are done for this range and
should continue to the next.
Reviewed by: markj, jhb (mentor)
Approved by: markj, jhb (mentor)
Differential Revision:https://reviews.freebsd.org/D24827
Modified:
head/sys/riscv/riscv/pmap.c
Modified: head/sys/riscv/riscv/pmap.c
==
--- head/sys/riscv/riscv/pmap.c Wed May 13 16:36:42 2020(r361009)
+++ head/sys/riscv/riscv/pmap.c Wed May 13 17:20:51 2020(r361010)
@@ -2329,6 +2329,7 @@ retryl2:
if (!atomic_fcmpset_long(l2, , l2e & ~mask))
goto retryl2;
anychanged = true;
+ continue;
} else {
if (!pv_lists_locked) {
pv_lists_locked = true;
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r360722 - head/sys/dev/virtio/mmio
On 7 May 2020, at 18:25, Li-Wen Hsu wrote: > On Wed, May 06, 2020 at 23:28:51 +0000, Jessica Clarke wrote: >> Author: jrtc27 >> Date: Wed May 6 23:28:51 2020 >> New Revision: 360722 >> URL: https://svnweb.freebsd.org/changeset/base/360722 >> >> Log: >> virtio_mmio: Support non-transitional version 2 devices >> >> The non-legacy virtio MMIO specification drops the use of PFNs and >> replaces them with physical addresses. Whilst many implementations are >> so-called transitional devices, also implementing the legacy >> specification, TinyEMU[1] does not. Device-specific configuration >> registers have also changed to being little-endian, and must be accessed >> using a single aligned access for registers up to 32 bits, and two >> 32-bit aligned accesses for 64-bit registers. >> >> [1] https://bellard.org/tinyemu/ >> >> Reviewed by:br, brooks (mentor) >> Approved by:br, brooks (mentor) >> Differential Revision: https://reviews.freebsd.org/D24681 >> >> Modified: >> head/sys/dev/virtio/mmio/virtio_mmio.c >> head/sys/dev/virtio/mmio/virtio_mmio.h > > Hi Jessica, > > It looks this commit breaks armv6 and armv7 builds: > > > --- virtio_mmio.o --- > /usr/src/sys/dev/virtio/mmio/virtio_mmio.c:442:13: error: shift count >= > width of type [-Werror,-Wshift-count-overflow] >paddr >> 32); > ^ ~~ > /usr/src/sys/dev/virtio/mmio/virtio_mmio.c:127:44: note: expanded from macro > 'vtmmio_write_config_4' >VIRTIO_MMIO_PREWRITE(sc->platform, (o), (v)); \ > ^ > ... > > https://ci.freebsd.org/job/FreeBSD-head-armv6-build/9109/console > https://ci.freebsd.org/job/FreeBSD-head-armv7-build/9035/console Thanks, yes, of course. This should be fixed as of r360789. Jess ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r360789 - head/sys/dev/virtio/mmio
Author: jrtc27 Date: Thu May 7 17:59:17 2020 New Revision: 360789 URL: https://svnweb.freebsd.org/changeset/base/360789 Log: virtio_mmio: Add casts missing from r360722 This fixes -Wshift-count-overflow warnings/errors on architectures using 32-bit physical addresses. Reported by: lwhsu Modified: head/sys/dev/virtio/mmio/virtio_mmio.c Modified: head/sys/dev/virtio/mmio/virtio_mmio.c == --- head/sys/dev/virtio/mmio/virtio_mmio.c Thu May 7 17:58:07 2020 (r360788) +++ head/sys/dev/virtio/mmio/virtio_mmio.c Thu May 7 17:59:17 2020 (r360789) @@ -439,19 +439,19 @@ vtmmio_set_virtqueue(struct vtmmio_softc *sc, struct v vtmmio_write_config_4(sc, VIRTIO_MMIO_QUEUE_DESC_LOW, paddr); vtmmio_write_config_4(sc, VIRTIO_MMIO_QUEUE_DESC_HIGH, - paddr >> 32); + ((uint64_t)paddr) >> 32); paddr = virtqueue_avail_paddr(vq); vtmmio_write_config_4(sc, VIRTIO_MMIO_QUEUE_AVAIL_LOW, paddr); vtmmio_write_config_4(sc, VIRTIO_MMIO_QUEUE_AVAIL_HIGH, - paddr >> 32); + ((uint64_t)paddr) >> 32); paddr = virtqueue_used_paddr(vq); vtmmio_write_config_4(sc, VIRTIO_MMIO_QUEUE_USED_LOW, paddr); vtmmio_write_config_4(sc, VIRTIO_MMIO_QUEUE_USED_HIGH, - paddr >> 32); + ((uint64_t)paddr) >> 32); vtmmio_write_config_4(sc, VIRTIO_MMIO_QUEUE_READY, 1); } ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r360723 - in head/sys/dev/virtio: balloon console random scsi
Author: jrtc27
Date: Wed May 6 23:31:30 2020
New Revision: 360723
URL: https://svnweb.freebsd.org/changeset/base/360723
Log:
virtio: Support MMIO bus for all devices
The bus is independent of the device, so all devices can be attached to
either a PCI bus or an MMIO bus. For example, QEMU's virtio-rng-device
gives the MMIO variant of virtio-rng-pci, and is now detected.
Reviewed by: andrew, br, brooks (mentor)
Approved by: andrew, br, brooks (mentor)
Differential Revision:https://reviews.freebsd.org/D24730
Modified:
head/sys/dev/virtio/balloon/virtio_balloon.c
head/sys/dev/virtio/console/virtio_console.c
head/sys/dev/virtio/random/virtio_random.c
head/sys/dev/virtio/scsi/virtio_scsi.c
Modified: head/sys/dev/virtio/balloon/virtio_balloon.c
==
--- head/sys/dev/virtio/balloon/virtio_balloon.cWed May 6 23:28:51
2020(r360722)
+++ head/sys/dev/virtio/balloon/virtio_balloon.cWed May 6 23:31:30
2020(r360723)
@@ -153,6 +153,8 @@ static driver_t vtballoon_driver = {
};
static devclass_t vtballoon_devclass;
+DRIVER_MODULE(virtio_balloon, virtio_mmio, vtballoon_driver,
+vtballoon_devclass, 0, 0);
DRIVER_MODULE(virtio_balloon, virtio_pci, vtballoon_driver,
vtballoon_devclass, 0, 0);
MODULE_VERSION(virtio_balloon, 1);
@@ -160,6 +162,7 @@ MODULE_DEPEND(virtio_balloon, virtio, 1, 1, 1);
VIRTIO_SIMPLE_PNPTABLE(virtio_balloon, VIRTIO_ID_BALLOON,
"VirtIO Balloon Adapter");
+VIRTIO_SIMPLE_PNPINFO(virtio_mmio, virtio_balloon);
VIRTIO_SIMPLE_PNPINFO(virtio_pci, virtio_balloon);
static int
Modified: head/sys/dev/virtio/console/virtio_console.c
==
--- head/sys/dev/virtio/console/virtio_console.cWed May 6 23:28:51
2020(r360722)
+++ head/sys/dev/virtio/console/virtio_console.cWed May 6 23:31:30
2020(r360723)
@@ -256,6 +256,8 @@ static driver_t vtcon_driver = {
};
static devclass_t vtcon_devclass;
+DRIVER_MODULE(virtio_console, virtio_mmio, vtcon_driver, vtcon_devclass,
+vtcon_modevent, 0);
DRIVER_MODULE(virtio_console, virtio_pci, vtcon_driver, vtcon_devclass,
vtcon_modevent, 0);
MODULE_VERSION(virtio_console, 1);
@@ -263,6 +265,7 @@ MODULE_DEPEND(virtio_console, virtio, 1, 1, 1);
VIRTIO_SIMPLE_PNPTABLE(virtio_console, VIRTIO_ID_CONSOLE,
"VirtIO Console Adapter");
+VIRTIO_SIMPLE_PNPINFO(virtio_mmio, virtio_console);
VIRTIO_SIMPLE_PNPINFO(virtio_pci, virtio_console);
static int
Modified: head/sys/dev/virtio/random/virtio_random.c
==
--- head/sys/dev/virtio/random/virtio_random.c Wed May 6 23:28:51 2020
(r360722)
+++ head/sys/dev/virtio/random/virtio_random.c Wed May 6 23:31:30 2020
(r360723)
@@ -96,6 +96,8 @@ static driver_t vtrnd_driver = {
};
static devclass_t vtrnd_devclass;
+DRIVER_MODULE(virtio_random, virtio_mmio, vtrnd_driver, vtrnd_devclass,
+vtrnd_modevent, 0);
DRIVER_MODULE(virtio_random, virtio_pci, vtrnd_driver, vtrnd_devclass,
vtrnd_modevent, 0);
MODULE_VERSION(virtio_random, 1);
@@ -104,6 +106,7 @@ MODULE_DEPEND(virtio_random, random_device, 1, 1, 1);
VIRTIO_SIMPLE_PNPTABLE(virtio_random, VIRTIO_ID_ENTROPY,
"VirtIO Entropy Adapter");
+VIRTIO_SIMPLE_PNPINFO(virtio_mmio, virtio_random);
VIRTIO_SIMPLE_PNPINFO(virtio_pci, virtio_random);
static int
Modified: head/sys/dev/virtio/scsi/virtio_scsi.c
==
--- head/sys/dev/virtio/scsi/virtio_scsi.c Wed May 6 23:28:51 2020
(r360722)
+++ head/sys/dev/virtio/scsi/virtio_scsi.c Wed May 6 23:31:30 2020
(r360723)
@@ -228,6 +228,8 @@ static driver_t vtscsi_driver = {
};
static devclass_t vtscsi_devclass;
+DRIVER_MODULE(virtio_scsi, virtio_mmio, vtscsi_driver, vtscsi_devclass,
+vtscsi_modevent, 0);
DRIVER_MODULE(virtio_scsi, virtio_pci, vtscsi_driver, vtscsi_devclass,
vtscsi_modevent, 0);
MODULE_VERSION(virtio_scsi, 1);
@@ -235,6 +237,7 @@ MODULE_DEPEND(virtio_scsi, virtio, 1, 1, 1);
MODULE_DEPEND(virtio_scsi, cam, 1, 1, 1);
VIRTIO_SIMPLE_PNPTABLE(virtio_scsi, VIRTIO_ID_SCSI, "VirtIO SCSI Adapter");
+VIRTIO_SIMPLE_PNPINFO(virtio_mmio, virtio_scsi);
VIRTIO_SIMPLE_PNPINFO(virtio_pci, virtio_scsi);
static int
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r360722 - head/sys/dev/virtio/mmio
Author: jrtc27
Date: Wed May 6 23:28:51 2020
New Revision: 360722
URL: https://svnweb.freebsd.org/changeset/base/360722
Log:
virtio_mmio: Support non-transitional version 2 devices
The non-legacy virtio MMIO specification drops the use of PFNs and
replaces them with physical addresses. Whilst many implementations are
so-called transitional devices, also implementing the legacy
specification, TinyEMU[1] does not. Device-specific configuration
registers have also changed to being little-endian, and must be accessed
using a single aligned access for registers up to 32 bits, and two
32-bit aligned accesses for 64-bit registers.
[1] https://bellard.org/tinyemu/
Reviewed by: br, brooks (mentor)
Approved by: br, brooks (mentor)
Differential Revision:https://reviews.freebsd.org/D24681
Modified:
head/sys/dev/virtio/mmio/virtio_mmio.c
head/sys/dev/virtio/mmio/virtio_mmio.h
Modified: head/sys/dev/virtio/mmio/virtio_mmio.c
==
--- head/sys/dev/virtio/mmio/virtio_mmio.c Wed May 6 23:23:22 2020
(r360721)
+++ head/sys/dev/virtio/mmio/virtio_mmio.c Wed May 6 23:28:51 2020
(r360722)
@@ -47,6 +47,7 @@ __FBSDID("$FreeBSD$");
#include
#include
#include
+#include
#include
#include
@@ -76,6 +77,8 @@ static intvtmmio_read_ivar(device_t, device_t, int, u
static int vtmmio_write_ivar(device_t, device_t, int, uintptr_t);
static uint64_tvtmmio_negotiate_features(device_t, uint64_t);
static int vtmmio_with_feature(device_t, uint64_t);
+static voidvtmmio_set_virtqueue(struct vtmmio_softc *sc,
+ struct virtqueue *vq, uint32_t size);
static int vtmmio_alloc_virtqueues(device_t, int, int,
struct vq_alloc_info *);
static int vtmmio_setup_intr(device_t, enum intr_type);
@@ -223,6 +226,16 @@ vtmmio_attach(device_t dev)
return (ENXIO);
}
+ sc->vtmmio_version = vtmmio_read_config_4(sc, VIRTIO_MMIO_VERSION);
+ if (sc->vtmmio_version < 1 || sc->vtmmio_version > 2) {
+ device_printf(dev, "Unsupported version: %x\n",
+ sc->vtmmio_version);
+ bus_release_resource(dev, SYS_RES_MEMORY, 0,
+ sc->res[0]);
+ sc->res[0] = NULL;
+ return (ENXIO);
+ }
+
vtmmio_reset(sc);
/* Tell the host we've noticed this device. */
@@ -404,6 +417,46 @@ vtmmio_with_feature(device_t dev, uint64_t feature)
return ((sc->vtmmio_features & feature) != 0);
}
+static void
+vtmmio_set_virtqueue(struct vtmmio_softc *sc, struct virtqueue *vq,
+uint32_t size)
+{
+ vm_paddr_t paddr;
+
+ vtmmio_write_config_4(sc, VIRTIO_MMIO_QUEUE_NUM, size);
+#if 0
+ device_printf(dev, "virtqueue paddr 0x%08lx\n",
+ (uint64_t)paddr);
+#endif
+ if (sc->vtmmio_version == 1) {
+ vtmmio_write_config_4(sc, VIRTIO_MMIO_QUEUE_ALIGN,
+ VIRTIO_MMIO_VRING_ALIGN);
+ paddr = virtqueue_paddr(vq);
+ vtmmio_write_config_4(sc, VIRTIO_MMIO_QUEUE_PFN,
+ paddr >> PAGE_SHIFT);
+ } else {
+ paddr = virtqueue_desc_paddr(vq);
+ vtmmio_write_config_4(sc, VIRTIO_MMIO_QUEUE_DESC_LOW,
+ paddr);
+ vtmmio_write_config_4(sc, VIRTIO_MMIO_QUEUE_DESC_HIGH,
+ paddr >> 32);
+
+ paddr = virtqueue_avail_paddr(vq);
+ vtmmio_write_config_4(sc, VIRTIO_MMIO_QUEUE_AVAIL_LOW,
+ paddr);
+ vtmmio_write_config_4(sc, VIRTIO_MMIO_QUEUE_AVAIL_HIGH,
+ paddr >> 32);
+
+ paddr = virtqueue_used_paddr(vq);
+ vtmmio_write_config_4(sc, VIRTIO_MMIO_QUEUE_USED_LOW,
+ paddr);
+ vtmmio_write_config_4(sc, VIRTIO_MMIO_QUEUE_USED_HIGH,
+ paddr >> 32);
+
+ vtmmio_write_config_4(sc, VIRTIO_MMIO_QUEUE_READY, 1);
+ }
+}
+
static int
vtmmio_alloc_virtqueues(device_t dev, int flags, int nvqs,
struct vq_alloc_info *vq_info)
@@ -448,15 +501,7 @@ vtmmio_alloc_virtqueues(device_t dev, int flags, int n
break;
}
- vtmmio_write_config_4(sc, VIRTIO_MMIO_QUEUE_NUM, size);
- vtmmio_write_config_4(sc, VIRTIO_MMIO_QUEUE_ALIGN,
- VIRTIO_MMIO_VRING_ALIGN);
-#if 0
- device_printf(dev, "virtqueue paddr 0x%08lx\n",
- (uint64_t)virtqueue_paddr(vq));
-#endif
- vtmmio_write_config_4(sc, VIRTIO_MMIO_QUEUE_PFN,
- virtqueue_paddr(vq) >> PAGE_SHIFT);
+ vtmmio_set_virtqueue(sc, vq, size);
vqx->vtv_vq = *info->vqai_vq = vq;
vqx->vtv_no_intr = info->vqai_intr == NULL;
@@ -568,10 +613,54 @@ vtmmio_read_dev_config(device_t
svn commit: r359682 - head/sys/riscv/riscv
Author: jrtc27
Date: Mon Apr 6 23:54:50 2020
New Revision: 359682
URL: https://svnweb.freebsd.org/changeset/base/359682
Log:
riscv: Add semicolon missing from r359672
Somehow this got lost between build-testing and submitting to Phabricator.
Modified:
head/sys/riscv/riscv/pmap.c
Modified: head/sys/riscv/riscv/pmap.c
==
--- head/sys/riscv/riscv/pmap.c Mon Apr 6 23:38:46 2020(r359681)
+++ head/sys/riscv/riscv/pmap.c Mon Apr 6 23:54:50 2020(r359682)
@@ -4354,7 +4354,7 @@ pmap_sync_icache(pmap_t pmap, vm_offset_t va, vm_size_
sched_pin();
mask = all_harts;
CPU_CLR(PCPU_GET(hart), );
- fence_i()
+ fence_i();
if (!CPU_EMPTY() && smp_started) {
fence();
sbi_remote_fence_i(mask.__bits);
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r359672 - head/sys/riscv/riscv
Author: jrtc27
Date: Mon Apr 6 22:31:30 2020
New Revision: 359672
URL: https://svnweb.freebsd.org/changeset/base/359672
Log:
riscv: Make sure local hart's icache is synced in pmap_sync_icache
The only way to flush the local hart's icache is with a FENCE.I (or an
equivalent SBI call); a normal FENCE is insufficient and, for the
single-hart case, unnecessary.
Reviewed by: jhb (mentor), markj
Approved by: jhb (mentor), markj
Differential Revision:https://reviews.freebsd.org/D24317
Modified:
head/sys/riscv/riscv/pmap.c
Modified: head/sys/riscv/riscv/pmap.c
==
--- head/sys/riscv/riscv/pmap.c Mon Apr 6 22:29:15 2020(r359671)
+++ head/sys/riscv/riscv/pmap.c Mon Apr 6 22:31:30 2020(r359672)
@@ -4335,13 +4335,20 @@ pmap_sync_icache(pmap_t pmap, vm_offset_t va, vm_size_
* RISC-V harts, the writing hart has to execute a data FENCE
* before requesting that all remote RISC-V harts execute a
* FENCE.I."
+*
+* However, this is slightly misleading; we still need to
+* perform a FENCE.I for the local hart, as FENCE does nothing
+* for its icache. FENCE.I alone is also sufficient for the
+* local hart.
*/
sched_pin();
mask = all_harts;
CPU_CLR(PCPU_GET(hart), );
- fence();
- if (!CPU_EMPTY() && smp_started)
+ fence_i()
+ if (!CPU_EMPTY() && smp_started) {
+ fence();
sbi_remote_fence_i(mask.__bits);
+ }
sched_unpin();
}
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r359671 - head/sys/riscv/riscv
Author: jrtc27
Date: Mon Apr 6 22:29:15 2020
New Revision: 359671
URL: https://svnweb.freebsd.org/changeset/base/359671
Log:
riscv: Fix pmap_fault_fixup for L3 pages
Summary:
The parentheses being in the wrong place means that, for L3 pages,
oldpte has all bits except PTE_V cleared, and so all the subsequent
checks against oldpte will fail, causing us to bail out and not retry
the faulting instruction after an SFENCE.VMA. This causes a WITNESS +
INVARIANTS kernel to fault on the "Chisel P3" (BOOM-based) DARPA SSITH
GFE SoC in pmap_init when writing to pv_table and, being a nofault
entry, subsequently panic with:
panic: vm_fault_lookup: fault on nofault entry, addr: 0xffc004e0
Reviewed by: markj
Approved by: markj
Differential Revision:https://reviews.freebsd.org/D24315
Modified:
head/sys/riscv/riscv/pmap.c
Modified: head/sys/riscv/riscv/pmap.c
==
--- head/sys/riscv/riscv/pmap.c Mon Apr 6 22:14:50 2020(r359670)
+++ head/sys/riscv/riscv/pmap.c Mon Apr 6 22:29:15 2020(r359671)
@@ -2416,7 +2416,7 @@ pmap_fault_fixup(pmap_t pmap, vm_offset_t va, vm_prot_
goto done;
if ((l2e & PTE_RWX) == 0) {
pte = pmap_l2_to_l3(l2, va);
- if (pte == NULL || ((oldpte = pmap_load(pte) & PTE_V)) == 0)
+ if (pte == NULL || ((oldpte = pmap_load(pte)) & PTE_V) == 0)
goto done;
} else {
pte = l2;
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
