Traditionally, struct virtio_device is shared between SLOF and C code.
This still remains shared with the addition of virtio_cap structure as
well. Now both virtio_device and virtio_cap structures are shared.
Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
virtio device structure carries a type variable indicating whether
virtio is over PCI or VIO. While VIO is not there and no plan to
introduce other transport, there is no purpose of having this variable
around and checking for PCI.
Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
MAC reading should be done after the initialization of the device after
the features negotiation.
Adjust the open routine accordingly. There is no point in sending the
mac address to the virtionet_open. Change the signature. Also read the
mac address directly from the config space to remove the dependency of
getting the mac address from the open routine.
Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Size of the net_hdr_size is different in legacy and modern devices. This
helps in conversion of the driver to 1.0.
Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Using an array here is not necassary, simplifies the code for
readability.
Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Simplifies the driver code and is helpful for migration to virtio 1.0
enablement.
Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
The virtio_set_status lines were getting too long because of OR'ing the
status on the same line of the call. Moreover, going forward we need to
add FEATURES_OK status as well. The state progress is quite straight
forward, so use status variable instead. Code looks cleaner and can easily
make out the change in the state.
Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Enable virtio_fill_desc/fill_blk_hdr with legacy and modern mode for
further use
Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
With the lack of the virtio_queue_init_vq routine, driver like
virtio-blk and virtio-9p had disabled device reset in the
initialization code. This helper will fix that problem, as the
initialization can be done after the device reset.
Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
The routine takes care to allocate and set the queue address in the
device. Add these calls in virtio-net, virtio-blk and virtio-9p.
With the lack of this routine, devices like virtio-blk and virtio-9p did
not do a device reset in the driver initialization code. This helper
will fix that problem
Change the signature of virtio_set_qaddr, accepting queue address as
unsigned long argument.
Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
The patch does not make any functional changes.
Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Change rx_size type which originally should have been uint32_t. This
also requires a change in the function prototype.
Also change the return type of virtio_9p_load() to silence another gcc
warning
Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Change the prototype of virtioblk_read as blocknum wont be negative
Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
This is an attempt to standartize the coding style in the project.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Reviewed-by: Thomas Huth <thuth@redhat.com>
There is another implementation in netapps/args.c which is used by
netboot.c and ping.c so switch to it.
Suggested-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Reviewed-by: Thomas Huth <thuth@redhat.com>
This fixes gcc warnings but unlike other places, this does not change
the type of bytes_received as recv() may return a negative value;
instead this adds:
1) casting to size_t when comparing the size;
2) an additional check for a negative value returned from recv().
The make command used to test:
make qemu WARNFLAGS=-Wextra
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Reviewed-by: Thomas Huth <thuth@redhat.com>
This replaces some local variable types and some function parameters from
signed to unsigned to fix gcc warnings.
Tested DHCP+TFTP on both IPv4 and IPv6.
The make command used to test:
make qemu WARNFLAGS=-Wextra
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Reviewed-by: Thomas Huth <thuth@redhat.com>
This restyles function prototypes:
- return types on the same line;
- opening brace is on the next line.
VIM configs used for this was:
set noexpandtab
set tabstop=8
set shiftwidth=4
set cino=:0,(0
This replaces [><]* with "*" as >< are also used to resolve merge
conflicts.
This removes trailing spaces.
This removes some redundant braces.
This should cause no behavioural change.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Reviewed-by: Thomas Huth <thuth@redhat.com>
The dhcp6_process_options() function allocates a struct
dhcp6_received_options each time it is called - but that
struct never gets used - and especially is also never freed
again. Fix this memory leak by simply removing the unused code.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
ip6_to_multicast_mac() uses a malloc to allocate a 6 bytes buffer
for the MAC address - which never gets freed again! That means
we're leaking memory for every multicast IPv6 packet that gets
send out. Fix it by simply using the "uint8_t mac[6]" array from
send_ipv6() instead.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
The code that checks for NEIGHBOUR_SOLICITATION packets is bad
in two ways: First, it does not check that the packet is really
an ICMPv6 packet, so it could accidentially be triggered for UDP
or TCP packets, too. Second, the fill_ethhdr() is redundant (since
it is done at the end of the function again) and also wrong (it
uses the wrong buffer pointer to fill in the ethernet header).
All we really need here is to get the right MAC address via
ip6_to_multicast_mac() - and this is handled in the following
if-statement already (NEIGHBOUR_SOLICITATIONs are always sent
as multicast in the send_neighbour_solicitation() function), so
the bad code block can simply be removed to fix the two issues.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Since slof/fs/dma-function.fs has been introduced for board-qemu,
these function definitions shadow the original js2x functions from
slof/fs/pci-config-bridge.fs, because dma-functions.fs is included
after pci-config-bridge.fs in slof/fs/pci-bridge.fs.
To restore the original behavior, move the js2x functions into
a board-specific dma-function.fs file instead and include that
one into the ROM-fs instead of the version for board-qemu.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
When manually specifying the TFTP server address and boot file name
in IPv6 mode, we can still get the client IPv6 address via link-local
or stateless IPv6 address autoconfiguration. So the netboot should
not abort if just the client IPv6 address has not been specified yet.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Using a memcpy to just compare one byte looks very cumbersome.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
The IPv6 code declares a bunch of global variables (without "extern"
keyword!) in the ipv6.h header file. This is bad style and does not
work when linking with "-fno-common" for example. So let's move
the variables to the files where they are used instead.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
When the IPv6 code is told to create IPv6 addresses automatically
(by passing NULL as parameter to set_ipv6_address()), the netboot
code currently only uses link-local IPv6 addresses - which is bad
since they can not be routed, e.g. if the TFTP server is not on
the same link.
So set_ipv6_address(NULL) should set own_ip6 preferably to a non-local
unicast address if it has been generated successfully during ipv6_init().
Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
The code that checks whether the router advertisments contain only
a link-local address has a bug: It must check the full first 10 bits
of the address, not only the first 9 bits. Otherwise, site-local
addresses (which start with 0xFEC0...) are also recognized as
link-local, which is wrong, of course.
Fix it by also introducing a proper wrapper functions for link-local
addresses (which will be used in a later patch, too).
Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
While looking at the network traffic from SLOF with Wireshark, I
noticed that there are some junk bytes at the end of TFTP ACK
packets. The problem is that send_ack() adds sizeof(struct ethhdr)
to the length of the packet that it wants to transmit. But adding
the sizeof(struct ethhdr) to the length of the packet is done
within send_ipv6() already, so this value got added twice.
Removing it from send_ack() fixes this issue - the packets then
look fine in Wireshark again.
send_error() apparently suffers from the same issue, so let's
also remove the sizeof(struct ethhdr) from that function.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
When compiling with OHCI_DEBUG enabled, gcc complains about a
lot of bad format strings. So let's use more appropriate format
modifiers to fix these warnings.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Wireshark reports bad FCS values and trailing zeros for SLOF's
DHCPv6 packets. This happens due to two bugs. First, the length
given to send_ipv6() contained sizeof(struct ethhdr) - but
adding that length is the responsibility of the send_ipv6()
function itself, the upper layers must not add this length yet.
Second, struct option_request was defined with 5 options, but
the code only uses 3 options instead. So when the code uses
sizeof(struct dhcp_message_header) (which contains the struct
option_request), there were additional unused bytes appended
to the message.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Generate a proper transaction ID before sending DHCPv4 packets
and check whether the DHCPv4 replies contain that correct XID, too.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Generate a proper transaction ID before sending DHCPv6 packets
and check whether the DHCPv6 replies contain that correct XID, too.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Use the MAC address and the current timebase value to seed the
pseudo-random number generator - this will hopefully give use
enough pseudo-randomness so that two guests that are booting in
parallel won't use the same rand() numbers.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Needed for seeding the state of the pseudo-random number generator.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
The rand() function in SLOF's libc has a bug which caused the
function to always return zero: The _rand value was shifted left
by 16, and then ANDed with 0x7fff. Obviously, the shift operation
should be ">>" instead of "<<".
And while we're at it, also increase the constant for the
multiplaction in there so that more upper bits are affected.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
The output of net-snk is bad in a couple of places:
1) The hard-coded "Bootloader 1.6" message is pretty useless
for the users (we never changed the version number in the
past five years) - so let's change that into printing
"Initializing NIC" instead.
2) "Requesting IP address via DHCP" is wrong for DHCPv6 since
in this case, the net-snk might only request the boot file
name via DHCPv6. In the IPv6-only case, the net-snk was
already printing "Requesting information..." instead, so
let's unify these texts to always print "information" instead
of "IP address"
3) The client IPv6 address is never printed out, so do that now,
too.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
The buffer size that we currently use for the client-architecture-
support call currently only works if there is not more than 1 TiB
of hot-pluggable RAM for the guest. This will likely not be enough
in the near future, so increase the buffer size from 128 kiB to 2 MiB
instead. That should be enough to accomodate for 16 TiB of hot-pluggable
memory.
While we're at it, also rename the "size" variable to something more
specific (since it is put into the namespace of the root node), and
add a proper error message in case the alloc-mem fails.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
The "hv-cas" hypercall is only available on board-qemu.
Including archsupport.fs on board-js2x breaks the boot
process there. Thus the archsupport.fs file should
reside in the board-qemu/slof directory instead and
only be included from there.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
QEMU when started with "-boot strict=on", appends "HALT" at the end of
the bootlist. Handle this in the boot order.
Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
The maximum number of root directory entries can be zero on
certain FAT32 file systems. In this case, we've got to read
the cluster number of the root directory from the extended BPB
instead. See also the following URL for details:
https://en.wikipedia.org/wiki/Design_of_the_FAT_file_system#BPB20_OFS_06h
Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
The Forth-to-C wrapper for get-named-nvram-partition also
uses the STRING_INIT macro. This causes heavy stack usage
in the engine() function due to the static array in that
macro. So let's rework the wrapper to do the string convertion
in a separate function instead.
Now that all users of the STRING_INIT and STRING_FROM_STACK
macros are gone, the macros can be removed, too.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
The Forth-to-C wrapper code in libnvram.code uses a temporary
buffer of 255 bytes via the STRING_INIT macro for each Forth string
that has to be converted to a C string. However, using such big
arrays in the wrapper code is a bad idea: Each of the buffers
is put into the stack frame of the engine() function (from paflof.c)!
That means the 7 strings from libnvram.code increase the stack
usage of engine() by 7 * 255 = 1785 bytes! This can cause stack
overflows since engine() can be called recursively, e.g. via the
forth_eval() macro.
To fix this issue in the functions from envvar.c, we can simply
pass the Forth strings directly to the functions by adding the
string length as additional function parameter, since the functions
in envvar.c don't really depend on NUL-terminated strings.
And while we're at it (i.e. we touch the function prototypes here
anyway), also rename the functions to have a proper "nvram_" prefix,
so we clearly mark them as part of libnvram instead of cluttering
the global name space with rather trivial function names.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Since initial port from slof to skiboot, vsnprintf() has improved in
skiboot so let's port the improved version back.
Suggested-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
The code from the FAST_RFILL macro uses a local array as a
temporary buffer - which gets allocated in the stack frame
of the engine() function. Since engine() can be called
recursively, this can cause stack overflows. So let's
move the rfill code into a separate function to avoid
these problems.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
The wrapper for new_nvram_partition() is using a 12 bytes buffer to
create a zero-terminated string. However, if the string has exactly
12 characters, the final NUL-terminator is missing. new_nvram_partition()
then calls create_nvram_partition() internally which depends on proper
NUL-terminated strings. So fix this by making sure that the copied
string is always NUL-terminated - and while we're at it, also move
the copy code out of libnvram.code to save some precious bytes in the
stack space of the engine() function.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
When scanning hubs, the code in libusb can be called recursively, for
example usb_hub_init() calls setup_new_device(), which then calls
slof_usb_handle() to execute Forth code for the next device.
If that next device is a hub, we end up recursively in usb_hub_init()
again.
Since stack space is limited in SLOF, we can optimize here a little
bit by splitting up the setup_new_device() function into the part
that retrieves the descriptors (which takes most of the stack space
in this code path since the descriptors are placed on the stack),
and the part that populates the the device tree node of the new
device (which is responsible for the recursion).
Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Since engine() can be called recursively, this function is
prone for causing stack overflows. Add a sanity check and
spill out an error message if we detect an overflow. It's
not a bullet-proof check, but still, it should catch at least
the larger overflows and thus hopefully help detecting and
debugging these overflow problems in the future.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Nikunj A Dadhania