The Windows virtio driver ignores the advertized seg_max field and
assumes the host can accept up to 67 segments in indirect descriptors,
triggering an assert in the bhyve process.
This brings back r282922 but with a couple of changes:
- It raises the block interface segment limit to 128 instead of 67.
- Linux's virtio driver assumes that the segment limit is no
larger than the ring size. To avoid breaking Linux guests,
raise the VirtIO ring size to 128, and cap the VirtIO segment
limit at ring size - 2 (effectively 126).
Reviewed by: rgrimes, Patrick Mooney <pmooney@pfmooney.com>
Obtained from: Joyent (Linux workaround)
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D18831
When the CPU Topology was added to bhyve in r332298 the SMBIOS table was
missed, this table passes topology information to the system and was still
using the old concept of each vCPU is a socket with 1 core and 1 thread.
This code did not even try to use the old sysctl information to adjust
this data.
Correct that by building a proper SMBios table, mapping the > 254 cases to
0 per the SMBios 2.6 specification that is claimed by the structure.
Reviewed by: Patrick Mooney <patrick.mooney@joyent.com>
Approved by: bde and/or phk (mentor), jhb (maintainer)
MFC: 3 days
Differential Revision: https://reviews.freebsd.org/D18998
The bhyve acpi MADT table was given a static space of 256 (0x100) bytes,
this is enough space to allow VM_MAXCPU to be 21, this patch changes that
so VM_MAXCPU can be of arbitrary value and not overflow the space by
actually calculating the space needed for the table.
PR: 212782
Reviewed by: Patrick Mooney <patrick.mooney@joyent.com>
Approved by: bde (mentor), jhb (maintainer)
MFC after: 3 days
Differential Revision: https://reviews.freebsd.org/D18815
bhyve was previously using stdin for both reading and writing to the
console, which made it difficult to redirect console output. Use
stdin for reading and stdout for writing. This makes it easier to use
bhyve as a backend for syzkaller.
As a side effect, the change fixes a minor bug which would cause bhyve
to fail with ENOTCAPABLE if configured to use nmdm for com1 and stdio
for com2.
bhyveload already uses separate descriptors, as does the bvmcons driver.
Reviewed by: jhb
MFC after: 1 month
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D19788
Backing out commit pending further discussion on the PCIe version
supported by pseudo (i.e. emulated) devices. See Differential for
details.
Reviewed by: imp
Approved by: imp (mentor)
Differential Revision: https://reviews.freebsd.org/D19580
DEBUG_FLAGS is always added to CFLAGS. This setting appears to be
accidental and came in with r243327.
Reviewed by: anish, emaste, jhb, rgrimes
MFC after: 1 week
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D19787
bhyve's NVMe emulation was transferring Identify data back to the guest
incorrectly causing memory corruptions. These corruptions resulted in
core dumps and other system level errors in the guest.
In their simplest form, NVMe Physical Region Page (PRP) values in
commands indicate which physical pages to use for data transfer. The
first PRP value is not required to be page aligned but does not cross a
page boundary. The second PRP value must be page aligned, does not cross
a page boundary, and need not be contiguous with PRP1.
The code was copying Identify data past the end of PRP1. This happens to
work if PRP1 and PRP2 are physically contiguous but will corrupt guest
memory in unpredictable ways if they are not.
Fix is to copy the Identify data back to the guest piecewise (i.e. for
each PRP entry). Also fix a similarly wrong problem when copying back
Log page data.
Reviewed by: imp (mentor), araujo, jhb, rgrimes, bhyve
Approved by: imp (mentor), bhyve (jhb)
MFC after: 2 weeks
Differential Revision: https://reviews.freebsd.org/D19695
The NVMe specification defines bits 13:4 of BAR0 as Reserved (i.e. 0x0).
Most drivers do not enforce this, but the Windows NVMe driver does and
will refuse to start the device (i.e. error 10) if any of these bits are
set.
The current BAR size calculation tries to minimize the amount of memory
the device reserves by scaling the BAR size by the maximum number of
queues supported by the device. But unless the device supports a large
number of queue pairs (over 1536), it will reserve too little memory.
The fix is to allocate a minimum of 16K bytes for BAR0.
Tested on Windows Server 2016 and 2019
Reviewed by: imp (mentor), araujo, jhb, bhyve
Approved by: imp (mentor), bhyve (jhb)
MFC after: 2 weeks
Differential Revision: https://reviews.freebsd.org/D19676
PCIe devices starting with version 1.1 must set the Role-Based Error
Reporting bit.
And while we're in the neighborhood, generalize the code assigning the
device type.
Reviewed by: imp, araujo, rgrimes
Approved by: imp (mentor)
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D19580
The NVMe Identify Namespace data structure's Number of LBA Formats
(NLBAF) field is a 0's based value (i.e. 0x0 means 1). Since the
emulation only supports a single format, set NLBAF to 0x0, not 1.
Reviewed by: imp, araujo, rgrimes
Approved by: imp (mentor)
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D19579
THRE is always asserted in LSR reads, so REG_IER writes that raise
IER_ETXRDY must also set thre_int_pending.
Reported by: Illumos, according to emaste@
https://twitter.com/ed_maste/status/1106195949087584258
MFC after: 2 weeks
was incorrectly implemented leading to a possible double free.
It is possible for both the conditional free,
and the unconditional free added in r340044 to be done,
fix that by initializing uopt to NULL,
removing the conditional free,
and only using the unconditional free at the end.
Reported by: Patrick Mooney (patrick.mooney@joyent.com)
Reviewed by: jhb (maintainer), Patrick Mooney (joyent/illumos)
Approved by: bde (mentor)
CID: 1357336
MFC after: 3 days
MFC with: 340044
Differential Revision: https://reviews.freebsd.org/D19202
nopt is the only allocated space,
xopt and cp are aliases into that allocated space.
Remove the 2 unneeded free's
Reported by: Patrick Mooney (@pmooney_pfmooney.com)
Reviewed by: jhb (maintainer), Patrick Mooney (joyent/illumos)
Approved by: bde (mentor)
CID: 1305412
MFC after: 3 days
MFC with: 340042
Differential Revision: https://reviews.freebsd.org/D19200
will return success when the kernel is built without support of
the capability mode.
It is important to note, that I'm taking a more conservative approach
with these changes and it will be done in small steps.
Reviewed by: jhb
MFC after: 6 weeks
Differential Revision: https://reviews.freebsd.org/D18744
The function which processes Admin commands was not returning the
Command Specific value in Completion Queue Entry, Dword 0 (CDW0). This
effects commands such as Set Features, Number of Queues which returns
the number of queues supported by the device in CDW0. In this case, the
host will only create 1 queue pair (Number of Queues is zero based).
This also masked a bug in the queue counting logic.
Reviewed by: imp, araujo
Approved by: imp (mentor)
MFC after: 1 month
Differential Revision: https://reviews.freebsd.org/D18703
Many size / length parameters in NVMe are "0's based", meaning, a value
of 0x0 represents 1, 0x1 represents 2, etc.. While this leads to an
efficient encoding, it can lead to subtle bugs. With respect to queues,
these parameters include:
- Maximum number of queue entries
- Maximum number of queues
- Number of Completion Queues
- Number of Submission Queues
To be consistent, convert all 0's based values from the host to 1's
based value internally. Likewise, covert internal 1's based values to
0's based values when returned to the host. This fixes an off-by-one bug
when creating IO queues and simplifies some of the code. Note that this
bug is masked by another bug.
While in the neighborhood,
- fix an erroneous queue ID check (checking CQ count when deleting SQ)
- check for queue ID of 0x0 in a few places where this is illegal
- clean up the Set Features, Number of Queues command and check for
illegal values
Reviewed by: imp, araujo
Approved by: imp (mentor)
MFC after: 1 month
Differential Revision: https://reviews.freebsd.org/D18702
There was a large refactoring done in CTL to allow multiple ioctl frontend
ports (and respective devices) to be created, particularly for bhyve.
Unfortunately, respective part of bhyve functionality got lost somehow from
the original virtio-scsi commit. This change allows wanted device path to
be specified in either of two ways:
-s 6,virtio-scsi,/dev/cam/ctl1.1
-s 6,virtio-scsi,dev=/dev/cam/ctl2.3
If neither is specified, the default /dev/cam/ctl device is used.
While there, remove per-queue CTL device opening, which makes no sense at
this point.
Reported by: wg
Reviewed by: araujo
MFC after: 3 days
Sponsored by: iXsystems, Inc.
Differential Revision: https://reviews.freebsd.org/D18504
- buf_to_iov() does not use buflen parameter, allowing out of bound read.
- buf_to_iov() leaks memory if seek argument > 0.
- iov_to_buf() doesn't need to reallocate buffer for every segment.
- there is no point to use size_t for iov counts, int is more then enough.
- some iov function arguments can be constified.
- pci_vtscsi_request_handle() used truncate_iov() incorrectly, allowing
getting out of buffer and possibly corrupting data.
- pci_vtscsi_controlq_notify() written returned status at wrong offset.
- pci_vtscsi_controlq_notify() leaked one buffer per event.
Reported by: wg
Reviewed by: araujo
MFC after: 1 week
Sponsored by: iXsystems, Inc.
Differential Revision: https://reviews.freebsd.org/D18465
Unfortunately ctl_scsi_zero_io() wipes that field, so it was always zero.
While there, targ_port is set by kernel, so user-space should not fill it.
MFC after: 1 week
This fixes an incomplete bounds check on the guest-supplied request
size where a very large request size could be interpreted as a negative
value and not be caught by the bounds check.
Submitted by: jhb
Reported by: Reno Robert
Approved by: so
Security: FreeBSD-SA-18:14.bhyve
Security: CVE-2018-17160
to avoid buffer accessed out of bounds, also switch to snprintf(3).
PR: 200859
Submitted by: Caglar <caglar@10ur.org>
Obtained from: https://github.com/mist64/xhyve/pull/24
MFC after: 4 weeks
Sponsored by: iXsystems Inc.
Also switch from int to size_t to keep portability.
Reviewed by: brooks
Sponsored by: iXsystems Inc.
Differential Revision: https://reviews.freebsd.org/D17795
Architectures Software Developer’s Manual Volume 3"). Add the document
to SEE ALSO in bhyve.8 (and pet manlint here a bit).
Reviewed by: jhb, rgrimes, 0mp
Approved by: kib (mentor)
Differential Revision: https://reviews.freebsd.org/D17531
Alternatively to IPv4 address:port this will allow to listen on IPv6
link-local (incl. scope), a specific address, or ::. Addresses have
to be given in RFC2732 format so that [::]:port parsing will work.
This patch also starts to introduce WITH_INET/INET6_SUPPORT to bhyve.
PR: 232018
Submitted by: Dave Rush (northwoodlogic.free gmail.com) (original)
Reviewed by: Dave Rush (updated verison)
MFC after: 3 days
For tools that uses bhyve such like libvirt, it is important to be able to
probe what features are supported by the given bhyve binary.
To give more context, libvirt probes bhyve's capabilities in a not very
effective way:
- Running 'bhyve -h' and parsing output.
- To detect devices, it runs 'bhyve -s 0,dev' for every each device and
parses error output to identify if the device is supported or not.
PR: 2101111
Submitted by: novel
MFC after: 2 weeks
Relnotes: yes
Sponsored by: iXsystems Inc.
The original NVMe API used bit-fields to represent fields in data
structures defined by the specification (e.g. the op-code in the command
data structure). The implementation targeted x86_64 processors and
defined the bit fields for little endian dwords (i.e. 32 bits).
This approach does not work as-is for big endian architectures and was
changed to use a combination of bit shifts and masks to support PowerPC.
Unfortunately, this changed the NVMe API and forces #ifdef's based on
the OS revision level in user space code.
This change reverts to something that looks like the original API, but
it uses bytes instead of bit-fields inside the packed command structure.
As a bonus, this works as-is for both big and little endian CPU
architectures.
Bump __FreeBSD_version to 1200081 due to API change
Reviewed by: imp, kbowling, smh, mav
Approved by: imp (mentor)
Differential Revision: https://reviews.freebsd.org/D16404
