This patch provides a caching mechanism for the device address
extensions uid and fid on S390. For efficient sparse address allocation,
we introduce two hash tables for uid/fid which hold the address set
information per domain. Also in order to improve performance of
searching available value, we introduce our own callbacks for the two
hashtables. In this way, uid/fid is saved in hash key and hash value
could be any non-NULL pointer due to no operation on hash value. That is
also the reason why we don't introduce hash value free callback.
Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
The struct is called virPCIDeviceAddress and the
functions operating on it should be named accordingly.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The affected functions are
virDeviceInfoPCIAddressWanted()
virDeviceInfoPCIAddressPresent()
which get renamed to
virDeviceInfoPCIAddressIsWanted()
virDeviceInfoPCIAddressIsPresent()
to comply with the naming convention used for other
predicates.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
SetCreate, SetAddControllers, Reserve
last uses of these functions outside domain_addr.c removed in commit:
40c284f0a6
Assign
never used outside domain_addr.c
move Assign and Reserve above their first call within domain_addr.c
Signed-off-by: Anya Harter <aharter@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Allocate, Validate, SetCreate
last uses of these functions outside domain_addr.c removed in commit:
7bdd06b4e1
Signed-off-by: Anya Harter <aharter@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
from src/qemu/qemu_domain_address.c to src/conf/domain_addr.c
and rename to virDomainCCWAddressSetCreateFromDomain
(rename to have Address in full instead of Addr to follow
the naming convention of other virDomainCCWAddress functions)
Signed-off-by: Anya Harter <aharter@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
I haven't been able to come up with a single scenario in which
the code in question would be executed; even if there was one,
it would be due to the user specifying a *partial* PCI topology
in the guest XML, which is of course entirely unsupportable and
thus providing even the slightest hint that doing so is in any
way a good idea is actively harmful.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Both pcie-to-pci-bridge and dmi-to-pci-bridge can be used to
create a traditional PCI topology in a pure PCIe guest such as
those using the x86_64/q35 or aarch64/virt machine type;
however, the former should be preferred, as it doesn't need to
obey limitation of real hardware and is completely
architecture-agnostic.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1520821
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The new controller will not yet be used automatically by
libvirt, but at this point it's already possible to configure
a guest to use it.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
We're going to add a similarly-named attribute later, and we'd
like to be consistent.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Clang 6.0.0 complains when initializing structure with { NULL }:
conf/domain_addr.c:1494:38: error: missing field 'type' initializer [-Werror,-Wmissing-field-initializers]
virDomainDeviceInfo nfo = { NULL };
Use { 0 } instead to make it happy.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Rather than having the caller check, if the input @addrs is NULL
(e.g. priv->usbaddrs), then just return 0. This also removes the
need for ATTRIBUTE_NONNULL which only really helped if someone
passed a NULL as a parameter not if the passed parameter is NULL.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Rather than having the caller check, if the input @addrs is NULL
(e.g. priv->usbaddrs), then just return 0. This also removes the
need for ATTRIBUTE_NONNULL which only really helped if someone
passed a NULL as a parameter not if the passed parameter is NULL.
Signed-off-by: John Ferlan <jferlan@redhat.com>
The controller model is slightly unusual in that the default value is
-1, not 0. As a result the default value is not covered by any of the
existing enum cases. This in turn means that any switch() statements
that think they have covered all cases, will in fact not match the
default value at all. In the qemuDomainDeviceCalculatePCIConnectFlags()
method this has caused a serious mistake where we fallthrough from the
SCSI controller case, to the VirtioSerial controller case, and from
the USB controller case to the IDE controller case.
By adding explicit enum constant starting at -1, we can ensure switches
remember to handle the default case.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
These rules will make it possible for libvirt to
automatically assign PCI addresses in a way that
respects any isolation constraints devices might
have.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Laine Stump <laine@laine.org>
Isolation groups will eventually allow us to make sure certain
devices, eg. PCI hostdevs, are assigned to guest PCI buses in
a way that guarantees improved isolation, error detection and
recovery for machine types and hypervisors that support it,
eg. pSeries guest on QEMU.
This patch merely defines storage for the new information
we're going to need later on and makes sure it is passed from
the hypervisor driver (QEMU / bhyve) down to the generic PCI
address allocation code.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Laine Stump <laine@laine.org>
When looking for slots suitable for a PCI device, libvirt
might need to add an extra PCI controller: for pSeries guests,
we want that extra controller to be a PHB (pci-root) rather
than a PCI bridge.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Laine Stump <laine@laine.org>
The current algorithm for slot allocation tries to be clever
and avoid looking at buses / slots more than once unless it's
necessary. Unfortunately that makes the code more complex,
and it will cause problem later on in some situations unless
even more complex code is added.
Since the performance gains are going to be pretty modest
anyway, we can just get rid of the extra complexity and use a
completely straighforward implementation instead.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Laine Stump <laine@laine.org>
Move @function after @flags to match other functions in the
same module like virDomainPCIAddressReserveNextAddr().
Also move virDomainPCIAddressReserveNextAddr() closer to
virDomainPCIAddressReserveAddr() in the header file.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Laine Stump <laine@laine.org>
This function was private to the QEMU driver and was,
accordingly, called qemuDomainPCIBusFullyReserved().
However the function is really not QEMU-specific at
all, so it makes sense to move it closer to the
virDomainPCIAddressBus struct it operates on.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Laine Stump <laine@laine.org>
When switching over the values in the virDomainControllerModelPCI
enumeration, make sure the proper cast is in place so that the
compiler can warn us when the coverage is not exaustive.
For the same reason, remove the 'default' case from one of the
existing switch statements.
The switch in virDomainPCIControllerModelToConnectType()
had some code that, while techically part of the
_PCIE_SWITCH_DOWNSTREAM_PORT case, was in fact dead due
to the early return.
Get rid of the dead code, and fix the inaccurate function
description while at it.
Move the range check introduced by commit 2650d5e into
virDomainUSBAddressFindPort. That way both virDomainUSBAddressRelease
and virDomainUSBAddressSetAddHub can benefit from it.
Reported-by: Michal Privoznik <mprivozn@redhat.com>
Surprisingly there was a virDomainPCIAddressReleaseAddr() function
already, but it was completely unused. Since we don't reserve entire
slots at once any more, there is no need to release entire slots
either, so we just replace the single call to
virDomainPCIAddressReleaseSlot() with a call to
virDomainPCIAddressReleaseAddr() and remove the now unused function.
The keen observer may be concerned that ...Addr() doesn't call
virDomainPCIAddressValidate(), as ...Slot() did. But really the
validation was pointless anyway - if the device hadn't been suitable
to be connected at that address, it would have failed validation
before every being reserved in the first place, so by definition it
will pass validation when it is being unplugged. (And anyway, even if
something "bad" happened and we managed to have a device incorrectly
at the given address, we would still want to be able to free it up for
use by a device that *did* validate properly).
This is in preparation for renaming virDomainPCIAddressReserveSlot()
to virDomainPCIAddressReserveAddr(), which is a better description of
what it does.
Since we don't actually reserve an entire slot at a time anymore, the
name of this function is just confusing, and it's almost identical in
operation to virDomainPCIAddressReserveNextAddr() anyway, so remove
the *Slot() function and replace calls to it with calls to *Addr(...,
-1).
With the advent of VIR_PCI_CONNECT_AGGREGATE_SLOT, the new name is
more appropriate, since the address returned may be another address
on the same slot as last time, not necessarily a new slot.
fromConfig should be true if the caller wants
virDomainPCIAddressValidate() to loosen restrictions on its
interpretation of the pciConnectFlags. In particular, either
PCI_DEVICE or PCIE_DEVICE will be counted as equivalent to both, and
HOTPLUG will be ignored. In a few cases where libvirt was manually
overriding automatic address assignment, it was setting fromConfig to
false when validating the hardcoded manual override. This patch
changes those to fromConfig=true as a preemptive strike against any
future bugs that might otherwise surface.
Although setting virDomainPCIAddressReserveAddr()'s fromConfig=true is
correct when a PCI addres is coming from a domain's config, the *true*
purpose of the fromConfig argument is to lower restrictions on what
kind of device can plug into what kind of controller - if fromConfig
is true, then a PCIE_DEVICE can plug into a slot that is marked as
only compatible with PCI_DEVICE (and vice versa), and the HOTPLUG flag
is ignored.
For a long time there have been several calls to
virDomainPCIAddressReserveAddr() that have fromConfig incorrectly set
to false - it's correct that the addresses aren't coming from user
config, but they are coming from hardcoded exceptions in libvirt that
should, if anything, pay *even less* attention to following the
pciConnectFlags (under the assumption that the libvirt programmer knew
what they were doing).
See commit b87703cf7 for an example of an actual bug caused by the
incorrect setting of the "fromConfig" argument to
virDomainPCIAddressReserveAddr(). Although they haven't resulted in
any reported bugs, this patch corrects all the other incorrect
settings of fromConfig in calls to virDomainPCIAddressReserveAddr().
Set the VIR_PCI_CONNECT_AGGREGATE_SLOT flag for pcie-root-ports so
that they will be assigned to all the functions on a slot.
Some qemu test case outputs had to be adjusted due to the
pcie-root-ports now being put on multiple functions.
If a PCI device has VIR_PCI_CONNECT_AGGREGATE_SLOT set in its
pciConnectFlags, then during address assignment we allow multiple
instances of this type of device to be auto-assigned to multiple
functions on the same device. A slot is used for aggregating multiple
devices only if the first device assigned to that slot had
VIR_PCI_CONNECT_AGGREGATE_SLOT set. but any device types that have
AGGREGATE_SLOT set might be mix/matched on the same slot.
(NB: libvirt should never set the AGGREGATE_SLOT flag for a device
type that might need to be hotplugged. Currently it is only planned
for pcie-root-port and possibly other PCI controller types, and none
of those are hotpluggable anyway)
There aren't yet any devices that use this flag. That will be in a
later patch.
This utility function iterates through all devices looking for any
with a PCI address that has function != 0 (which implies that multiple
functions are in use on that slot), then uses an inner iterator to
find the device that's on function 0 of that same slot and sets the
"multi" in its virDomainDeviceInfo (as long as it hasn't already been
set explicitly by someone who presumably has better information than
we do).
It isn't yet called from anywhere, so will have no functional effect.
There is a very slight time advantage to beginning the search for the
next unused PCI address at the slot *after* the previous find (which
is now used), but if we do that, we will miss allocating the other
functions of the same slot (when we implement a
VIR_PCI_CONNECT_AGGREGATE_SLOT flag to support that).
virDomainPCIAddressGetNextSlot() starts searching from the last
allocated address and goes to the end of all the buses, then goes back
to the first bus and searches from there up to the starting point (in
case any address has been freed since the last time an address was
allocated. The loops for these two are almost, but not exactly, the
same, so they have remained as separate loops with the same code
inside the loop. To lessen maintenance headaches, the identical code
has been moved out into the function
virDomainPCIAddressFindUnusedFunctionOnBus(), which is called in place
of the loop contents.
setting reserveEntireSlot really accomplishes nothing - instead of
going to the trouble of computing the value for reserveEntireSlot and
then possibly setting *all* functions of the slot as in-use, we can
just set the in-use bit only for the specific function being used by a
device. Later we will know from the context (the PCI connect flags,
and whether we are reserving a specific address or asking for "the
next available") whether or not it is okay to allocate other functions
on the same slot.
Although it's not used yet, we allow specifying "-1" for the function
number when looking for the "next available slot" - this is going to
end up meaning "return the lowest available function in the slot, but
since we currently only provide a function from an otherwise unused
slot, "-1" ends up meaning "0".
