Re: [osv-dev] OSv fully and officially supported on Firecracker !!!

2019-03-05 Thread Pekka Enberg
On Wed, Mar 6, 2019 at 6:24 AM Waldek Kozaczuk  wrote:

> With couple of last patches applied yesterday we finally made OSv properly
> and fully run on Firecracker. This includes ability to run arbitrary images
> (zfs, rofs or ramfs) with networking and SMP supported as well.
>

Nice work, Waldek!

- Pekka

-- 
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to osv-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[osv-dev] OSv fully and officially supported on Firecracker !!!

2019-03-05 Thread Waldek Kozaczuk
With couple of last patches applied yesterday we finally made OSv properly 
and fully run on Firecracker. This includes ability to run arbitrary images 
(zfs, rofs or ramfs) with networking and SMP supported as well.

Many thanks to all those that made it happen. Especially to Nadav for 
relentlessly reviewing all the patches I produced and submitted and to 
Pekka for initial review and suggestions about my first patches.

Also I have added new Wiki page 
- https://github.com/cloudius-systems/osv/wiki/Running-OSv-on-Firecracker - 
that should help anyone interested to try OSv on firecracker.

Enjoy,
Waldek

PS. Hopefully sometime this week I should be cutting new release 0.53.0 of 
OSv that will obviously incorporate firecracker support.

-- 
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to osv-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[osv-dev] [COMMIT osv master] Fix another aarch64 compilation error

2019-03-05 Thread Commit Bot

From: Waldemar Kozaczuk 
Committer: Waldemar Kozaczuk 
Branch: master

Fix another aarch64 compilation error

Signed-off-by: Waldemar Kozaczuk 

---
diff --git a/drivers/virtio-blk.cc b/drivers/virtio-blk.cc
--- a/drivers/virtio-blk.cc
+++ b/drivers/virtio-blk.cc
@@ -144,11 +144,13 @@ blk::blk(virtio_device& virtio_dev)
 [=] { t->wake(); });
 };

+#ifndef AARCH64_PORT_STUB
 int_factory.create_gsi_edge_interrupt = [this,t]() {
 return new gsi_edge_interrupt(
 _dev.get_irq(),
 [=] { if (this->ack_irq()) t->wake(); });
 };
+#endif

 _dev.register_interrupt(int_factory);

diff --git a/drivers/virtio-net.cc b/drivers/virtio-net.cc
--- a/drivers/virtio-net.cc
+++ b/drivers/virtio-net.cc
@@ -317,11 +317,13 @@ net::net(virtio_device& dev)
 [=] { poll_task->wake(); });
 };

+#ifndef AARCH64_PORT_STUB
 int_factory.create_gsi_edge_interrupt = [this,poll_task]() {
 return new gsi_edge_interrupt(
 _dev.get_irq(),
 [=] { if (this->ack_irq()) poll_task->wake(); });
 };
+#endif

 _dev.register_interrupt(int_factory);

--
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to osv-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[osv-dev] [COMMIT osv master] Tweak firecracker.py to disable rate limiting for network device

2019-03-05 Thread Commit Bot

From: Waldemar Kozaczuk 
Committer: Waldemar Kozaczuk 
Branch: master

Tweak firecracker.py to disable rate limiting for network device

Signed-off-by: Waldemar Kozaczuk 

---
diff --git a/scripts/firecracker.py b/scripts/firecracker.py
--- a/scripts/firecracker.py
+++ b/scripts/firecracker.py
@@ -53,7 +53,27 @@ def add_network_interface(self, interface_name,  
host_interface_name, ):

 self.make_put_call('/network-interfaces/%s' % interface_name, {
 'iface_id': interface_name,
 'host_dev_name': host_interface_name,
-'guest_mac': "52:54:00:12:34:56"
+'guest_mac': "52:54:00:12:34:56",
+'rx_rate_limiter': {
+   'bandwidth': {
+  'size': 0,
+  'refill_time': 0
+   },
+   'ops': {
+  'size': 0,
+  'refill_time': 0
+   }
+},
+'tx_rate_limiter': {
+   'bandwidth': {
+  'size': 0,
+  'refill_time': 0
+   },
+   'ops': {
+  'size': 0,
+  'refill_time': 0
+   }
+}
 })

 def start_instance(self):

--
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to osv-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[osv-dev] [COMMIT osv master] Tweaked firecracker.py to not set disk to a root device

2019-03-05 Thread Commit Bot

From: Waldemar Kozaczuk 
Committer: Waldemar Kozaczuk 
Branch: master

Tweaked firecracker.py to not set disk to a root device

This patch effectively makes firecracker not to
add 'root=/dev/vda' to command line and makes interactive apps
like rogue or python2x/python3x work properly.

Signed-off-by: Waldemar Kozaczuk 

---
diff --git a/scripts/firecracker.py b/scripts/firecracker.py
--- a/scripts/firecracker.py
+++ b/scripts/firecracker.py
@@ -45,7 +45,7 @@ def add_disk(self, disk_image_path):
 self.make_put_call('/drives/rootfs', {
 'drive_id': 'rootfs',
 'path_on_host': disk_image_path,
-'is_root_device': True,
+'is_root_device': False,
 'is_read_only': False
 })

--
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to osv-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[osv-dev] [COMMIT osv master] Fix firecracker.py to pass input to OSv

2019-03-05 Thread Commit Bot

From: Waldemar Kozaczuk 
Committer: Waldemar Kozaczuk 
Branch: master

Fix firecracker.py to pass input to OSv

Fixes #1028

Signed-off-by: Waldemar Kozaczuk 

---
diff --git a/scripts/firecracker.py b/scripts/firecracker.py
--- a/scripts/firecracker.py
+++ b/scripts/firecracker.py
@@ -150,8 +150,7 @@ def start_firecracker(firecracker_path, socket_path):

 # Start firecracker process to communicate over specified UNIX socket  
file

 return subprocess.Popen([firecracker_path, '--api-sock', socket_path],
-   stdin=subprocess.PIPE, stdout=sys.stdout,
-   stderr=subprocess.STDOUT)
+   stdout=sys.stdout, stderr=subprocess.STDOUT)


 def get_memory_size_in_mb(options):

--
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to osv-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: [osv-dev] Re: [PATCH] Enhance SMP logic to parse MP table

2019-03-05 Thread Waldek Kozaczuk


On Tuesday, March 5, 2019 at 10:37:35 AM UTC-5, Waldek Kozaczuk wrote:
>
>
>
> On Tuesday, March 5, 2019 at 7:53:38 AM UTC-5, Nadav Har'El wrote:
>>
>>
>> On Tue, Mar 5, 2019 at 2:33 PM Waldek Kozaczuk  
>> wrote:
>>
>>> Nadav,
>>>
>>> Thanks for reviewing the other patches
>>>
>>
>> Thank you, for doing this work!
>>
> Thanks for reviewing my numerous patches! 
>
>>  
>>
>>> . It looks like we are down to this single one to have full support of 
>>> OSv on firecracker.
>>>
>>
>> I committed this patch too, but something appears to have been broken 
>> with the command line handling. For example:
>>
>> $  scripts/build image=rogue
>> $ scripts/firecracker.py -v
>>
>> /home/nyh/osv.tmp2/osv$ scripts/firecracker.py -c2 -V  
>> 2019-03-05T14:47:40.768877: Start
>> 2019-03-05T14:47:40.771144: Firecracker ready
>> 2019-03-05T14:47:40.773686: Configured VM
>> 2019-03-05T14:47:40.775023: Added disk
>> 2019-03-05T14:47:40.776185: Created OSv VM with cmdline: --verbose 
>> --nopci /usr/games/rogue
>> 2019-03-05T14:47:40.782907: Booted OSv VM
>> 2019-03-05T14:47:40.782938: Waiting for firecracker process to terminate
>> OSv v0.52.0-48-g47ae2b65
>> 2 CPUs detected
>> Firmware vendor: Unknown
>> bsd: initializing - done
>> VFS: mounting ramfs at /
>> VFS: mounting devfs at /dev
>> net: initializing - done
>> Detected virtio-mmio device: (2,0)
>> virtio-blk: Add blk device instances 0 as vblk0, devsize=0
>> random: intel drng, rdrand registered as a source.
>> random:  initialized
>> VFS: unmounting /dev
>> VFS: mounting rofs at /rofs
>> failed to mount /rofs, error = No error information
>> VFS: mounting zfs at /zfs
>> zfs: mounting osv/zfs from device /dev/vblk0.1
>> VFS: mounting devfs at /dev
>> VFS: mounting procfs at /proc
>> program zpool.so returned 1
>> BSD shrinker: event handler list found: 0xa0f16f00
>> BSD shrinker found: 1
>> BSD shrinker: unlocked, running
>> 2019-03-05T14:47:40.839064594 [anonymous-instance:WARN:vmm/src/
>> lib.rs:1080] Guest-boot-time =  62091 us 62 ms,  69244 CPU us 69 CPU ms
>> *root=/dev/vda: No such file or directory*
>> program exited with status 1
>> VFS: unmounting /dev
>> VFS: unmounting /proc
>> VFS: unmounting /
>> Powering off.
>> 2019-03-05T14:47:40.848183837 [anonymous-instance:ERROR:vmm/src/
>> lib.rs:1320] Failed to log metrics while stopping: Logger was not 
>> initialized.
>> 2019-03-05T14:47:40.862006: End
>>
>>
>> I don't where this message comes from, I am guessing that something got 
>> broken in the command line and OSv tried to run the string "root=/dev/vda" 
>> as the command line instead of the desired command line (/usr/games/rogue)?
>> Using build fs=ramfs does not help, so it seems the new virtio stuff is 
>> working, just something in the command line got broken?
>>
> Indeed the issue is with the command line and I am actually not surprised. 
> I very rarely use rogue as I am not really familiar with this game ;-) 
> Never came across it in the 80ties. 
>
> I any case firecracker appends some Linux-specific parameters to the 
> command line (see here for command line example - 
> https://github.com/firecracker-microvm/firecracker/releases - default 
> "reboot=k 
> panic=1 pci=off nomodules 8250.nr_uarts=0 i8042.noaux i8042.nomux 
> i8042.nopnp i8042.dumbkbd") that we do not care for except for 
> virtio-mmio stuff which we parse and remove from command line before we 
> pass it on the downstream logic. The applications that I have used to test 
> OSv on firecracker (*-hello, *-httpserver ones for example) do not care 
> about extra parameters but in the rogue case it somehow cares. The 
> particular one (*root=/dev/vda*) I think comes from the fact that adding 
> block device makes it add this parameter to command line. I think the rouge 
> would boot properly with previous version of firecracker.py where it did 
> not add block device to firecracker instance. BTW it would be nice to 
> change OSv to show received command line in verbose mode. I would find it 
> very handy.
>
> Long story short in order to address these command line woes, we need to 
> come up with some sort of 'OSv command line marker' scheme. For example we 
> could add logic to look for some special end of command line sequence of 
> characters or add new boot option called '--cmdline_end_marker' which would 
> explicitly state what to look for end of the command line. Do you have 
> other ideas?
>
> Relatedly you may have noticed that new firecracker.py automatically 
> converts usr.img to usr.raw as firecracker cannot deal with qcow :-( This 
> is not a problem with rofs images (which btw I typically use with 
> firecracker as it boots much faster) but leads to 10G usr.raw file when we 
> use zfs. Obviously one can pass fs_size_mb parameter to build script but I 
> wonder if there is a better way to handle it. Change default 10G to 
> something smaller? I tried to qemu-img resize the image but it would make 
> unbootable. Do you have any ideas?
>
> Finally I saw you 

Re: [osv-dev] Re: [PATCH] Enhance SMP logic to parse MP table

2019-03-05 Thread Waldek Kozaczuk


On Tuesday, March 5, 2019 at 7:53:38 AM UTC-5, Nadav Har'El wrote:
>
>
> On Tue, Mar 5, 2019 at 2:33 PM Waldek Kozaczuk  > wrote:
>
>> Nadav,
>>
>> Thanks for reviewing the other patches
>>
>
> Thank you, for doing this work!
>
Thanks for reviewing my numerous patches! 

>  
>
>> . It looks like we are down to this single one to have full support of 
>> OSv on firecracker.
>>
>
> I committed this patch too, but something appears to have been broken with 
> the command line handling. For example:
>
> $  scripts/build image=rogue
> $ scripts/firecracker.py -v
>
> /home/nyh/osv.tmp2/osv$ scripts/firecracker.py -c2 -V  
> 2019-03-05T14:47:40.768877: Start
> 2019-03-05T14:47:40.771144: Firecracker ready
> 2019-03-05T14:47:40.773686: Configured VM
> 2019-03-05T14:47:40.775023: Added disk
> 2019-03-05T14:47:40.776185: Created OSv VM with cmdline: --verbose --nopci 
> /usr/games/rogue
> 2019-03-05T14:47:40.782907: Booted OSv VM
> 2019-03-05T14:47:40.782938: Waiting for firecracker process to terminate
> OSv v0.52.0-48-g47ae2b65
> 2 CPUs detected
> Firmware vendor: Unknown
> bsd: initializing - done
> VFS: mounting ramfs at /
> VFS: mounting devfs at /dev
> net: initializing - done
> Detected virtio-mmio device: (2,0)
> virtio-blk: Add blk device instances 0 as vblk0, devsize=0
> random: intel drng, rdrand registered as a source.
> random:  initialized
> VFS: unmounting /dev
> VFS: mounting rofs at /rofs
> failed to mount /rofs, error = No error information
> VFS: mounting zfs at /zfs
> zfs: mounting osv/zfs from device /dev/vblk0.1
> VFS: mounting devfs at /dev
> VFS: mounting procfs at /proc
> program zpool.so returned 1
> BSD shrinker: event handler list found: 0xa0f16f00
> BSD shrinker found: 1
> BSD shrinker: unlocked, running
> 2019-03-05T14:47:40.839064594 [anonymous-instance:WARN:vmm/src/lib.rs:1080] 
> Guest-boot-time =  62091 us 62 ms,  69244 CPU us 69 CPU ms
> *root=/dev/vda: No such file or directory*
> program exited with status 1
> VFS: unmounting /dev
> VFS: unmounting /proc
> VFS: unmounting /
> Powering off.
> 2019-03-05T14:47:40.848183837 [anonymous-instance:ERROR:vmm/src/
> lib.rs:1320] Failed to log metrics while stopping: Logger was not 
> initialized.
> 2019-03-05T14:47:40.862006: End
>
>
> I don't where this message comes from, I am guessing that something got 
> broken in the command line and OSv tried to run the string "root=/dev/vda" 
> as the command line instead of the desired command line (/usr/games/rogue)?
> Using build fs=ramfs does not help, so it seems the new virtio stuff is 
> working, just something in the command line got broken?
>
Indeed the issue is with the command line and I am actually not surprised. 
I very rarely use rogue as I am not really familiar with this game ;-) 
Never came across it in the 80ties. 

I any case firecracker appends some Linux-specific parameters to the 
command line (see here for command line example 
- https://github.com/firecracker-microvm/firecracker/releases - default 
"reboot=k 
panic=1 pci=off nomodules 8250.nr_uarts=0 i8042.noaux i8042.nomux 
i8042.nopnp i8042.dumbkbd") that we do not care for except for virtio-mmio 
stuff which we parse and remove from command line before we pass it on the 
downstream logic. The applications that I have used to test OSv on 
firecracker (*-hello, *-httpserver ones for example) do not care about 
extra parameters but in the rogue case it somehow cares. The particular one 
(*root=/dev/vda*) I think comes from the fact that adding block device 
makes it add this parameter to command line. I think the rouge would boot 
properly with previous version of firecracker.py where it did not add block 
device to firecracker instance. BTW it would be nice to change OSv to show 
received command line in verbose mode. I would find it very handy.

Long story short in order to address these command line woes, we need to 
come up with some sort of 'OSv command line marker' scheme. For example we 
could add logic to look for some special end of command line sequence of 
characters or add new boot option called '--cmdline_end_marker' which would 
explicitly state what to look for end of the command line. Do you have 
other ideas?

Relatedly you may have noticed that new firecracker.py automatically 
converts usr.img to usr.raw as firecracker cannot deal with qcow :-( This 
is not a problem with rofs images (which btw I typically use with 
firecracker as it boots much faster) but leads to 10G usr.raw file when we 
use zfs. Obviously one can pass fs_size_mb parameter to build script but I 
wonder if there is a better way to handle it. Change default 10G to 
something smaller? I tried to qemu-img resize the image but it would make 
unbootable. Do you have any ideas?

Finally I saw you added new issue - 
https://github.com/cloudius-systems/osv/issues/1028 - about keyboard 
behavior on firecracker but given firecracker has very limited support for 
it 

Re: [osv-dev] [PATCH V2] Add virtio mmio implementation

2019-03-05 Thread Nadav Har'El
On Tue, Mar 5, 2019 at 2:57 PM Nadav Har'El  wrote:

> Hmm, I think I have accidentally committed an older version of this
> patch... Can you please check and incrementally commit the differences
> between the versions? Sorry about that.
>

I'm even more confused than that :-) I think that *you* are the one who
committed this patch, not me. And it seems you did commit the right one.
Sorry about the spam.

-- 
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to osv-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: [osv-dev] [PATCH V2] Add virtio mmio implementation

2019-03-05 Thread Nadav Har'El
Hmm, I think I have accidentally committed an older version of this
patch... Can you please check and incrementally commit the differences
between the versions? Sorry about that.

--
Nadav Har'El
n...@scylladb.com


On Mon, Mar 4, 2019 at 12:15 AM Waldemar Kozaczuk 
wrote:

> Adds implementation of virtio mmio devices
> needed to support block and networking devices
> on firecracker.
>
> Signed-off-by: Waldemar Kozaczuk 
> ---
>  Makefile   |   1 +
>  arch/x64/arch-setup.cc |   5 +
>  drivers/virtio-mmio.cc | 216 +
>  drivers/virtio-mmio.hh | 154 +
>  4 files changed, 376 insertions(+)
>  create mode 100644 drivers/virtio-mmio.cc
>  create mode 100644 drivers/virtio-mmio.hh
>
> diff --git a/Makefile b/Makefile
> index 377e93e9..3f560807 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -821,6 +821,7 @@ drivers += arch/$(arch)/pvclock-abi.o
>  drivers += drivers/virtio.o
>  drivers += drivers/virtio-pci-device.o
>  drivers += drivers/virtio-vring.o
> +drivers += drivers/virtio-mmio.o
>  drivers += drivers/virtio-net.o
>  drivers += drivers/vmxnet3.o
>  drivers += drivers/vmxnet3-queues.o
> diff --git a/arch/x64/arch-setup.cc b/arch/x64/arch-setup.cc
> index 7350714c..9317dd25 100644
> --- a/arch/x64/arch-setup.cc
> +++ b/arch/x64/arch-setup.cc
> @@ -24,9 +24,11 @@
>
>  osv_multiboot_info_type* osv_multiboot_info;
>
> +#include "drivers/virtio-mmio.hh"
>  void parse_cmdline(multiboot_info_type& mb)
>  {
>  auto p = reinterpret_cast(mb.cmdline);
> +virtio::parse_mmio_device_configuration(p);
>  osv::parse_cmdline(p);
>  }
>
> @@ -246,6 +248,9 @@ void arch_init_drivers()
>  boot_time.event("pci enumerated");
>  }
>
> +// Register any parsed virtio-mmio devices
> +virtio::register_mmio_devices(device_manager::instance());
> +
>  // Initialize all drivers
>  hw::driver_manager* drvman = hw::driver_manager::instance();
>  drvman->register_driver(virtio::blk::probe);
> diff --git a/drivers/virtio-mmio.cc b/drivers/virtio-mmio.cc
> new file mode 100644
> index ..12bf3413
> --- /dev/null
> +++ b/drivers/virtio-mmio.cc
> @@ -0,0 +1,216 @@
> +/*
> + * Copyright (C) 2019 Waldemar Kozaczuk
> + *
> + * This work is open source software, licensed under the terms of the
> + * BSD license as described in the LICENSE file in the top-level
> directory.
> + */
> +
> +#include 
> +#include 
> +#include "virtio-mmio.hh"
> +
> +namespace virtio {
> +
> +// This implements virtio-io mmio device (transport layer, modeled after
> PSI).
> +// Read here -
> https://www.kraxel.org/virtio/virtio-v1.0-cs03-virtio-gpu.html#x1-1080002
> +hw_device_id mmio_device::get_id()
> +{
> +return hw_device_id(0x1af4, _device_id);
> +}
> +
> +u8 mmio_device::get_status()
> +{
> +return mmio_getl(_addr_mmio + VIRTIO_MMIO_STATUS) & 0xff;
> +}
> +
> +void mmio_device::set_status(u8 status)
> +{
> +mmio_setl(_addr_mmio + VIRTIO_MMIO_STATUS, status);
> +}
> +
> +u64 mmio_device::get_available_features()
> +{
> +u64 features;
> +
> +mmio_setl(_addr_mmio + VIRTIO_MMIO_DEVICE_FEATURES_SEL, 1);
> +features = mmio_getl(_addr_mmio + VIRTIO_MMIO_DEVICE_FEATURES);
> +features <<= 32;
> +
> +mmio_setl(_addr_mmio + VIRTIO_MMIO_DEVICE_FEATURES_SEL, 0);
> +features |= mmio_getl(_addr_mmio + VIRTIO_MMIO_DEVICE_FEATURES);
> +
> +return features;
> +}
> +
> +void mmio_device::set_enabled_features(u64 features)
> +{
> +mmio_setl(_addr_mmio + VIRTIO_MMIO_DRIVER_FEATURES_SEL, 1);
> +mmio_setl(_addr_mmio + VIRTIO_MMIO_DRIVER_FEATURES, (u32)(features >>
> 32));
> +
> +mmio_setl(_addr_mmio + VIRTIO_MMIO_DRIVER_FEATURES_SEL, 0);
> +mmio_setl(_addr_mmio + VIRTIO_MMIO_DRIVER_FEATURES, (u32)features);
> +}
> +
> +void mmio_device::kick_queue(int queue_num)
> +{
> +mmio_setl(_addr_mmio + VIRTIO_MMIO_QUEUE_NOTIFY, queue_num);
> +}
> +
> +void mmio_device::select_queue(int queue_num)
> +{
> +mmio_setl(_addr_mmio + VIRTIO_MMIO_QUEUE_SEL, queue_num);
> +assert(!mmio_getl(_addr_mmio + VIRTIO_MMIO_QUEUE_READY));
> +}
> +
> +u16 mmio_device::get_queue_size()
> +{
> +return mmio_getl(_addr_mmio + VIRTIO_MMIO_QUEUE_NUM_MAX) & 0x;
> +}
> +
> +void mmio_device::setup_queue(vring* queue)
> +{
> +// Set size
> +mmio_setl(_addr_mmio + VIRTIO_MMIO_QUEUE_NUM, queue->size());
> +//
> +// Pass addresses
> +mmio_setl(_addr_mmio + VIRTIO_MMIO_QUEUE_DESC_LOW,
> (u32)queue->get_desc_addr());
> +mmio_setl(_addr_mmio + VIRTIO_MMIO_QUEUE_DESC_HIGH,
> (u32)(queue->get_desc_addr() >> 32));
> +
> +mmio_setl(_addr_mmio + VIRTIO_MMIO_QUEUE_AVAIL_LOW,
> (u32)queue->get_avail_addr());
> +mmio_setl(_addr_mmio + VIRTIO_MMIO_QUEUE_AVAIL_HIGH,
> (u32)(queue->get_avail_addr() >> 32));
> +
> +mmio_setl(_addr_mmio + VIRTIO_MMIO_QUEUE_USED_LOW,
> (u32)queue->get_used_addr());
> +mmio_setl(_addr_mmio + VIRTIO_MMIO_QUEUE_USED_HIGH,
> (u32)(queue->get_used_addr() >> 32));
> +}
> +
> 

Re: [osv-dev] Re: [PATCH] Enhance SMP logic to parse MP table

2019-03-05 Thread Nadav Har'El
On Tue, Mar 5, 2019 at 2:33 PM Waldek Kozaczuk  wrote:

> Nadav,
>
> Thanks for reviewing the other patches
>

Thank you, for doing this work!


> . It looks like we are down to this single one to have full support of OSv
> on firecracker.
>

I committed this patch too, but something appears to have been broken with
the command line handling. For example:

$  scripts/build image=rogue
$ scripts/firecracker.py -v

/home/nyh/osv.tmp2/osv$ scripts/firecracker.py -c2 -V
2019-03-05T14:47:40.768877: Start
2019-03-05T14:47:40.771144: Firecracker ready
2019-03-05T14:47:40.773686: Configured VM
2019-03-05T14:47:40.775023: Added disk
2019-03-05T14:47:40.776185: Created OSv VM with cmdline: --verbose --nopci
/usr/games/rogue
2019-03-05T14:47:40.782907: Booted OSv VM
2019-03-05T14:47:40.782938: Waiting for firecracker process to terminate
OSv v0.52.0-48-g47ae2b65
2 CPUs detected
Firmware vendor: Unknown
bsd: initializing - done
VFS: mounting ramfs at /
VFS: mounting devfs at /dev
net: initializing - done
Detected virtio-mmio device: (2,0)
virtio-blk: Add blk device instances 0 as vblk0, devsize=0
random: intel drng, rdrand registered as a source.
random:  initialized
VFS: unmounting /dev
VFS: mounting rofs at /rofs
failed to mount /rofs, error = No error information
VFS: mounting zfs at /zfs
zfs: mounting osv/zfs from device /dev/vblk0.1
VFS: mounting devfs at /dev
VFS: mounting procfs at /proc
program zpool.so returned 1
BSD shrinker: event handler list found: 0xa0f16f00
BSD shrinker found: 1
BSD shrinker: unlocked, running
2019-03-05T14:47:40.839064594 [anonymous-instance:WARN:vmm/src/lib.rs:1080]
Guest-boot-time =  62091 us 62 ms,  69244 CPU us 69 CPU ms
*root=/dev/vda: No such file or directory*
program exited with status 1
VFS: unmounting /dev
VFS: unmounting /proc
VFS: unmounting /
Powering off.
2019-03-05T14:47:40.848183837 [anonymous-instance:ERROR:vmm/src/lib.rs:1320]
Failed to log metrics while stopping: Logger was not initialized.
2019-03-05T14:47:40.862006: End


I don't where this message comes from, I am guessing that something got
broken in the command line and OSv tried to run the string "root=/dev/vda"
as the command line instead of the desired command line (/usr/games/rogue)?
Using build fs=ramfs does not help, so it seems the new virtio stuff is
working, just something in the command line got broken?


> Waldek
>
> On Saturday, March 2, 2019 at 1:30:52 AM UTC-5, Waldek Kozaczuk wrote:
>>
>> Adds logic to parse information about CPUs on
>> system where ACPI is not available. It does it
>> by parsing so called MP table. If MP table
>> not found assumes single vCPU.
>>
>> Signed-off-by: Waldemar Kozaczuk 
>> ---
>>  arch/x64/smp.cc | 125 ++--
>>  1 file changed, 111 insertions(+), 14 deletions(-)
>>
>> diff --git a/arch/x64/smp.cc b/arch/x64/smp.cc
>> index 2869d6e2..3386bd90 100644
>> --- a/arch/x64/smp.cc
>> +++ b/arch/x64/smp.cc
>> @@ -39,6 +39,16 @@ volatile unsigned smp_processors = 1;
>>
>>  using boost::intrusive::get_parent_from_member;
>>
>> +static void register_cpu(unsigned cpu_id, u32 apic_id, u32 acpi_id = 0)
>> +{
>> +auto c = new sched::cpu(cpu_id);
>> +c->arch.apic_id = apic_id;
>> +c->arch.acpi_id = acpi_id;
>> +c->arch.initstack.next = smp_stack_free;
>> +smp_stack_free = >arch.initstack;
>> +sched::cpus.push_back(c);
>> +}
>> +
>>  void parse_madt()
>>  {
>>  char madt_sig[] = ACPI_SIG_MADT;
>> @@ -57,12 +67,7 @@ void parse_madt()
>>  if (!(lapic->LapicFlags & ACPI_MADT_ENABLED)) {
>>  break;
>>  }
>> -auto c = new sched::cpu(nr_cpus++);
>> -c->arch.apic_id = lapic->Id;
>> -c->arch.acpi_id = lapic->ProcessorId;
>> -c->arch.initstack.next = smp_stack_free;
>> -smp_stack_free = >arch.initstack;
>> -sched::cpus.push_back(c);
>> +register_cpu(nr_cpus++, lapic->Id, lapic->ProcessorId);
>>  break;
>>  }
>>  default:
>> @@ -73,16 +78,108 @@ void parse_madt()
>>  debug(fmt("%d CPUs detected\n") % nr_cpus);
>>  }
>>
>> +#define MPF_IDENTIFIER (('_'<<24) | ('P'<<16) | ('M'<<8) | '_')
>> +struct mpf_structure {
>> +char signature[4];
>> +uint32_t configuration_table;
>> +uint8_t length;// In 16 bytes (e.g. 1 = 16 bytes, 2 = 32 bytes)
>> +uint8_t specification_revision;
>> +uint8_t checksum;  // This value should make all bytes in the table
>> equal 0 when added together
>> +uint8_t default_configuration; // If this is not zero then
>> configuration_table should be
>> +   // ignored and a default
>> configuration should be loaded instead
>> +uint32_t features; // If bit 7 is then the IMCR is present and PIC
>> mode is being used, otherwise
>> +   // virtual wire mode is; all other bits are
>> reserved
>> +} __attribute__((packed));
>> +
>> +#define MP_TABLE_IDENTIFIER 

[osv-dev] [COMMIT osv master] Enhance SMP logic to parse MP table

2019-03-05 Thread Commit Bot

From: Waldemar Kozaczuk 
Committer: Nadav Har'El 
Branch: master

Enhance SMP logic to parse MP table

Adds logic to parse information about CPUs on
system where ACPI is not available. It does it
by parsing so called MP table. If MP table
not found assumes single vCPU.

Signed-off-by: Waldemar Kozaczuk 
Message-Id: <20190302063049.2203-1-jwkozac...@gmail.com>

---
diff --git a/arch/x64/smp.cc b/arch/x64/smp.cc
--- a/arch/x64/smp.cc
+++ b/arch/x64/smp.cc
@@ -39,6 +39,16 @@ volatile unsigned smp_processors = 1;

 using boost::intrusive::get_parent_from_member;

+static void register_cpu(unsigned cpu_id, u32 apic_id, u32 acpi_id = 0)
+{
+auto c = new sched::cpu(cpu_id);
+c->arch.apic_id = apic_id;
+c->arch.acpi_id = acpi_id;
+c->arch.initstack.next = smp_stack_free;
+smp_stack_free = >arch.initstack;
+sched::cpus.push_back(c);
+}
+
 void parse_madt()
 {
 char madt_sig[] = ACPI_SIG_MADT;
@@ -57,12 +67,7 @@ void parse_madt()
 if (!(lapic->LapicFlags & ACPI_MADT_ENABLED)) {
 break;
 }
-auto c = new sched::cpu(nr_cpus++);
-c->arch.apic_id = lapic->Id;
-c->arch.acpi_id = lapic->ProcessorId;
-c->arch.initstack.next = smp_stack_free;
-smp_stack_free = >arch.initstack;
-sched::cpus.push_back(c);
+register_cpu(nr_cpus++, lapic->Id, lapic->ProcessorId);
 break;
 }
 default:
@@ -73,16 +78,108 @@ void parse_madt()
 debug(fmt("%d CPUs detected\n") % nr_cpus);
 }

+#define MPF_IDENTIFIER (('_'<<24) | ('P'<<16) | ('M'<<8) | '_')
+struct mpf_structure {
+char signature[4];
+uint32_t configuration_table;
+uint8_t length;// In 16 bytes (e.g. 1 = 16 bytes, 2 = 32 bytes)
+uint8_t specification_revision;
+uint8_t checksum;  // This value should make all bytes in the table  
equal 0 when added together
+uint8_t default_configuration; // If this is not zero then  
configuration_table should be
+   // ignored and a default configuration  
should be loaded instead
+uint32_t features; // If bit 7 is then the IMCR is present and PIC  
mode is being used, otherwise

+   // virtual wire mode is; all other bits are reserved
+} __attribute__((packed));
+
+#define MP_TABLE_IDENTIFIER (('P'<<24) | ('M'<<16) | ('C'<<8) | 'P')
+struct mp_table {
+char signature[4]; // "PCMP"
+uint16_t length;
+uint8_t mp_specification_revision;
+uint8_t checksum;  // Again, the byte should be all bytes in the table  
add up to 0

+char oem_id[8];
+char product_id[12];
+uint32_t oem_table;
+uint16_t oem_table_size;
+uint16_t entry_count;   // This value represents how many entries are  
following this table
+uint32_t lapic_address; // This is the memory mapped address of the  
local APICs

+uint16_t extended_table_length;
+uint8_t extended_table_checksum;
+uint8_t reserved;
+} __attribute__((packed));
+
+struct mp_processor {
+uint8_t type;  // Always 0
+uint8_t local_apic_id;
+uint8_t local_apic_version;
+uint8_t flags; // If bit 0 is clear then the processor must be ignored
+   // If bit 1 is set then the processor is the bootstrap  
processor

+uint32_t signature;
+uint32_t feature_flags;
+uint64_t reserved;
+} __attribute__((packed));
+
+static mp_table *find_mp_table(unsigned long base, long length)
+{
+// First find MP floating pointer structure in the physical memory
+// region specified by the base and length
+void *addr = mmu::phys_to_virt(base);
+while (length > 0) {
+   if (*static_cast(addr) == MPF_IDENTIFIER) {
+   // We found the MP floating pointer structure
+   auto mpf_struct = static_cast(addr);
+   // Now let us dereference physical address of MP table itself,
+   // check signature and return its virtual address
+   void *mp_table_addr =  
mmu::phys_to_virt(mpf_struct->configuration_table);
+   if (*static_cast(mp_table_addr) ==  
MP_TABLE_IDENTIFIER) {

+   return static_cast(mp_table_addr);
+   }
+   else {
+   return nullptr;
+   }
+   }
+
+   addr += 16;
+   length -= 16;
+}
+return nullptr;
+}
+
+#define LAST_KB_IN_BASE_MEMORY_ADDR  639 * 0x400
+#define FIRST_KB_IN_BASE_MEMORY_ADDR 0x0
+#define NON_PROCESSOR_ENTRY_SIZE 8
 void parse_mp_table()
 {
-//TODO: This a nasty hack to support single vCPU. Eventually we should
-// parse out equivalent information about all vCPUs from MP table. For
-// details please see  
https://wiki.osdev.org/Symmetric_Multiprocessing#Finding_information_using_MP_Table

-auto c = new sched::cpu(0);
-c->arch.apic_id = 0;
-c->arch.initstack.next = smp_stack_free;
-smp_stack_free = >arch.initstack;
-sched::cpus.push_back(c);
+// Parse information about all vCPUs from MP table. For 

[osv-dev] Re: [PATCH] Enhance SMP logic to parse MP table

2019-03-05 Thread Waldek Kozaczuk
Nadav,

Thanks for reviewing the other patches. It looks like we are down to this 
single one to have full support of OSv on firecracker.

Waldek

On Saturday, March 2, 2019 at 1:30:52 AM UTC-5, Waldek Kozaczuk wrote:
>
> Adds logic to parse information about CPUs on 
> system where ACPI is not available. It does it 
> by parsing so called MP table. If MP table 
> not found assumes single vCPU. 
>
> Signed-off-by: Waldemar Kozaczuk  
> --- 
>  arch/x64/smp.cc | 125 ++-- 
>  1 file changed, 111 insertions(+), 14 deletions(-) 
>
> diff --git a/arch/x64/smp.cc b/arch/x64/smp.cc 
> index 2869d6e2..3386bd90 100644 
> --- a/arch/x64/smp.cc 
> +++ b/arch/x64/smp.cc 
> @@ -39,6 +39,16 @@ volatile unsigned smp_processors = 1; 
>   
>  using boost::intrusive::get_parent_from_member; 
>   
> +static void register_cpu(unsigned cpu_id, u32 apic_id, u32 acpi_id = 0) 
> +{ 
> +auto c = new sched::cpu(cpu_id); 
> +c->arch.apic_id = apic_id; 
> +c->arch.acpi_id = acpi_id; 
> +c->arch.initstack.next = smp_stack_free; 
> +smp_stack_free = >arch.initstack; 
> +sched::cpus.push_back(c); 
> +} 
> + 
>  void parse_madt() 
>  { 
>  char madt_sig[] = ACPI_SIG_MADT; 
> @@ -57,12 +67,7 @@ void parse_madt() 
>  if (!(lapic->LapicFlags & ACPI_MADT_ENABLED)) { 
>  break; 
>  } 
> -auto c = new sched::cpu(nr_cpus++); 
> -c->arch.apic_id = lapic->Id; 
> -c->arch.acpi_id = lapic->ProcessorId; 
> -c->arch.initstack.next = smp_stack_free; 
> -smp_stack_free = >arch.initstack; 
> -sched::cpus.push_back(c); 
> +register_cpu(nr_cpus++, lapic->Id, lapic->ProcessorId); 
>  break; 
>  } 
>  default: 
> @@ -73,16 +78,108 @@ void parse_madt() 
>  debug(fmt("%d CPUs detected\n") % nr_cpus); 
>  } 
>   
> +#define MPF_IDENTIFIER (('_'<<24) | ('P'<<16) | ('M'<<8) | '_') 
> +struct mpf_structure { 
> +char signature[4]; 
> +uint32_t configuration_table; 
> +uint8_t length;// In 16 bytes (e.g. 1 = 16 bytes, 2 = 32 bytes) 
> +uint8_t specification_revision; 
> +uint8_t checksum;  // This value should make all bytes in the table 
> equal 0 when added together 
> +uint8_t default_configuration; // If this is not zero then 
> configuration_table should be 
> +   // ignored and a default configuration 
> should be loaded instead 
> +uint32_t features; // If bit 7 is then the IMCR is present and PIC 
> mode is being used, otherwise 
> +   // virtual wire mode is; all other bits are 
> reserved 
> +} __attribute__((packed)); 
> + 
> +#define MP_TABLE_IDENTIFIER (('P'<<24) | ('M'<<16) | ('C'<<8) | 'P') 
> +struct mp_table { 
> +char signature[4]; // "PCMP" 
> +uint16_t length; 
> +uint8_t mp_specification_revision; 
> +uint8_t checksum;  // Again, the byte should be all bytes in the 
> table add up to 0 
> +char oem_id[8]; 
> +char product_id[12]; 
> +uint32_t oem_table; 
> +uint16_t oem_table_size; 
> +uint16_t entry_count;   // This value represents how many entries are 
> following this table 
> +uint32_t lapic_address; // This is the memory mapped address of the 
> local APICs 
> +uint16_t extended_table_length; 
> +uint8_t extended_table_checksum; 
> +uint8_t reserved; 
> +} __attribute__((packed)); 
> + 
> +struct mp_processor { 
> +uint8_t type;  // Always 0 
> +uint8_t local_apic_id; 
> +uint8_t local_apic_version; 
> +uint8_t flags; // If bit 0 is clear then the processor must be 
> ignored 
> +   // If bit 1 is set then the processor is the bootstrap 
> processor 
> +uint32_t signature; 
> +uint32_t feature_flags; 
> +uint64_t reserved; 
> +} __attribute__((packed)); 
> + 
> +static mp_table *find_mp_table(unsigned long base, long length) 
> +{ 
> +// First find MP floating pointer structure in the physical memory 
> +// region specified by the base and length 
> +void *addr = mmu::phys_to_virt(base); 
> +while (length > 0) { 
> +   if (*static_cast(addr) == MPF_IDENTIFIER) { 
> +   // We found the MP floating pointer structure 
> +   auto mpf_struct = static_cast(addr); 
> +   // Now let us dereference physical address of MP table itself, 
> +   // check signature and return its virtual address 
> +   void *mp_table_addr = 
> mmu::phys_to_virt(mpf_struct->configuration_table); 
> +   if (*static_cast(mp_table_addr) == 
> MP_TABLE_IDENTIFIER) { 
> +   return static_cast(mp_table_addr); 
> +   } 
> +   else { 
> +   return nullptr; 
> +   } 
> +   } 
> + 
> +   addr += 16; 
> +   length -= 16; 
> +} 
> +return nullptr; 
> +} 
> + 
> +#define LAST_KB_IN_BASE_MEMORY_ADDR  639 * 0x400 
> +#define 

[osv-dev] [COMMIT osv master] Fixed typo in virtio-mmio code

2019-03-05 Thread Commit Bot

From: Waldemar Kozaczuk 
Committer: Waldemar Kozaczuk 
Branch: master

Fixed typo in virtio-mmio code

Signed-off-by: Waldemar Kozaczuk 

---
diff --git a/drivers/virtio-mmio.cc b/drivers/virtio-mmio.cc
--- a/drivers/virtio-mmio.cc
+++ b/drivers/virtio-mmio.cc
@@ -11,7 +11,7 @@

 namespace virtio {

-// This implements virtio-io mmio device (transport layer, modeled after  
PSI).
+// This implements virtio-io mmio device (transport layer, modeled after  
PCI).
 // Read here -  
https://www.kraxel.org/virtio/virtio-v1.0-cs03-virtio-gpu.html#x1-1080002

 hw_device_id mmio_device::get_id()
 {

--
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to osv-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[osv-dev] [COMMIT osv master] Add virtio mmio implementation

2019-03-05 Thread Commit Bot

From: Waldemar Kozaczuk 
Committer: Waldemar Kozaczuk 
Branch: master

Add virtio mmio implementation

Adds implementation of virtio mmio devices
needed to support block and networking devices
on firecracker.

Signed-off-by: Waldemar Kozaczuk 

---
diff --git a/Makefile b/Makefile
--- a/Makefile
+++ b/Makefile
@@ -821,6 +821,7 @@ drivers += arch/$(arch)/pvclock-abi.o
 drivers += drivers/virtio.o
 drivers += drivers/virtio-pci-device.o
 drivers += drivers/virtio-vring.o
+drivers += drivers/virtio-mmio.o
 drivers += drivers/virtio-net.o
 drivers += drivers/vmxnet3.o
 drivers += drivers/vmxnet3-queues.o
diff --git a/arch/x64/arch-setup.cc b/arch/x64/arch-setup.cc
--- a/arch/x64/arch-setup.cc
+++ b/arch/x64/arch-setup.cc
@@ -24,9 +24,11 @@

 osv_multiboot_info_type* osv_multiboot_info;

+#include "drivers/virtio-mmio.hh"
 void parse_cmdline(multiboot_info_type& mb)
 {
 auto p = reinterpret_cast(mb.cmdline);
+virtio::parse_mmio_device_configuration(p);
 osv::parse_cmdline(p);
 }

@@ -246,6 +248,9 @@ void arch_init_drivers()
 boot_time.event("pci enumerated");
 }

+// Register any parsed virtio-mmio devices
+virtio::register_mmio_devices(device_manager::instance());
+
 // Initialize all drivers
 hw::driver_manager* drvman = hw::driver_manager::instance();
 drvman->register_driver(virtio::blk::probe);
diff --git a/drivers/virtio-mmio.cc b/drivers/virtio-mmio.cc
--- a/drivers/virtio-mmio.cc
+++ b/drivers/virtio-mmio.cc
@@ -0,0 +1,216 @@
+/*
+ * Copyright (C) 2019 Waldemar Kozaczuk
+ *
+ * This work is open source software, licensed under the terms of the
+ * BSD license as described in the LICENSE file in the top-level directory.
+ */
+
+#include 
+#include 
+#include "virtio-mmio.hh"
+
+namespace virtio {
+
+// This implements virtio-io mmio device (transport layer, modeled after  
PSI).
+// Read here -  
https://www.kraxel.org/virtio/virtio-v1.0-cs03-virtio-gpu.html#x1-1080002

+hw_device_id mmio_device::get_id()
+{
+return hw_device_id(0x1af4, _device_id);
+}
+
+u8 mmio_device::get_status()
+{
+return mmio_getl(_addr_mmio + VIRTIO_MMIO_STATUS) & 0xff;
+}
+
+void mmio_device::set_status(u8 status)
+{
+mmio_setl(_addr_mmio + VIRTIO_MMIO_STATUS, status);
+}
+
+u64 mmio_device::get_available_features()
+{
+u64 features;
+
+mmio_setl(_addr_mmio + VIRTIO_MMIO_DEVICE_FEATURES_SEL, 1);
+features = mmio_getl(_addr_mmio + VIRTIO_MMIO_DEVICE_FEATURES);
+features <<= 32;
+
+mmio_setl(_addr_mmio + VIRTIO_MMIO_DEVICE_FEATURES_SEL, 0);
+features |= mmio_getl(_addr_mmio + VIRTIO_MMIO_DEVICE_FEATURES);
+
+return features;
+}
+
+void mmio_device::set_enabled_features(u64 features)
+{
+mmio_setl(_addr_mmio + VIRTIO_MMIO_DRIVER_FEATURES_SEL, 1);
+mmio_setl(_addr_mmio + VIRTIO_MMIO_DRIVER_FEATURES, (u32)(features >>  
32));

+
+mmio_setl(_addr_mmio + VIRTIO_MMIO_DRIVER_FEATURES_SEL, 0);
+mmio_setl(_addr_mmio + VIRTIO_MMIO_DRIVER_FEATURES, (u32)features);
+}
+
+void mmio_device::kick_queue(int queue_num)
+{
+mmio_setl(_addr_mmio + VIRTIO_MMIO_QUEUE_NOTIFY, queue_num);
+}
+
+void mmio_device::select_queue(int queue_num)
+{
+mmio_setl(_addr_mmio + VIRTIO_MMIO_QUEUE_SEL, queue_num);
+assert(!mmio_getl(_addr_mmio + VIRTIO_MMIO_QUEUE_READY));
+}
+
+u16 mmio_device::get_queue_size()
+{
+return mmio_getl(_addr_mmio + VIRTIO_MMIO_QUEUE_NUM_MAX) & 0x;
+}
+
+void mmio_device::setup_queue(vring* queue)
+{
+// Set size
+mmio_setl(_addr_mmio + VIRTIO_MMIO_QUEUE_NUM, queue->size());
+//
+// Pass addresses
+mmio_setl(_addr_mmio + VIRTIO_MMIO_QUEUE_DESC_LOW,  
(u32)queue->get_desc_addr());
+mmio_setl(_addr_mmio + VIRTIO_MMIO_QUEUE_DESC_HIGH,  
(u32)(queue->get_desc_addr() >> 32));

+
+mmio_setl(_addr_mmio + VIRTIO_MMIO_QUEUE_AVAIL_LOW,  
(u32)queue->get_avail_addr());
+mmio_setl(_addr_mmio + VIRTIO_MMIO_QUEUE_AVAIL_HIGH,  
(u32)(queue->get_avail_addr() >> 32));

+
+mmio_setl(_addr_mmio + VIRTIO_MMIO_QUEUE_USED_LOW,  
(u32)queue->get_used_addr());
+mmio_setl(_addr_mmio + VIRTIO_MMIO_QUEUE_USED_HIGH,  
(u32)(queue->get_used_addr() >> 32));

+}
+
+void mmio_device::activate_queue(int queue)
+{   //
+// Make it ready
+select_queue(queue);
+mmio_setl(_addr_mmio + VIRTIO_MMIO_QUEUE_READY, 1 );
+}
+
+u8 mmio_device::read_and_ack_isr()
+{
+unsigned long status = mmio_getl(_addr_mmio +  
VIRTIO_MMIO_INTERRUPT_STATUS);

+mmio_setl(_addr_mmio + VIRTIO_MMIO_INTERRUPT_ACK, status);
+return (status & VIRTIO_MMIO_INT_VRING);
+}
+
+u8 mmio_device::read_config(u32 offset)
+{
+return mmio_getb(_addr_mmio + VIRTIO_MMIO_CONFIG + offset);
+}
+
+void mmio_device::register_interrupt(interrupt_factory irq_factory)
+{
+_irq.reset(irq_factory.create_gsi_edge_interrupt());
+}
+
+bool mmio_device::parse_config()
+{
+_addr_mmio = mmio_map(_dev_info._address, _dev_info._size);
+
+u32 magic = mmio_getl(_addr_mmio + VIRTIO_MMIO_MAGIC_VALUE);
+if (magic != ('v' | 'i' << 8 | 'r' 

Re: [osv-dev] [PATCH] Add virtio mmio implementation

2019-03-05 Thread Nadav Har'El
On Sat, Mar 2, 2019 at 8:28 AM Waldemar Kozaczuk 
wrote:

> Adds implementation of virtio mmio devices
> needed to support block and networking devices
> on firecracker.
>
> Signed-off-by: Waldemar Kozaczuk 
> ---
>  Makefile   |   1 +
>  arch/x64/arch-setup.cc |   5 +
>  drivers/virtio-mmio.cc | 213 +
>  drivers/virtio-mmio.hh | 151 +
>  4 files changed, 370 insertions(+)
>  create mode 100644 drivers/virtio-mmio.cc
>  create mode 100644 drivers/virtio-mmio.hh
>
> diff --git a/Makefile b/Makefile
> index ff9f2ed3..b905063c 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -821,6 +821,7 @@ drivers += arch/$(arch)/pvclock-abi.o
>  drivers += drivers/virtio.o
>  drivers += drivers/virtio-pci-device.o
>  drivers += drivers/virtio-vring.o
> +drivers += drivers/virtio-mmio.o
>  drivers += drivers/virtio-net.o
>  drivers += drivers/vmxnet3.o
>  drivers += drivers/vmxnet3-queues.o
> diff --git a/arch/x64/arch-setup.cc b/arch/x64/arch-setup.cc
> index 6db2f77c..5ac9af1f 100644
> --- a/arch/x64/arch-setup.cc
> +++ b/arch/x64/arch-setup.cc
> @@ -63,9 +63,11 @@ struct e820ent {
>
>  osv_multiboot_info_type* osv_multiboot_info;
>
> +#include "drivers/virtio-mmio.hh"
>  void parse_cmdline(multiboot_info_type& mb)
>  {
>  auto p = reinterpret_cast(mb.cmdline);
> +virtio::parse_mmio_device_configuration(p);
>  osv::parse_cmdline(p);
>  }
>
> @@ -368,6 +370,9 @@ void arch_init_drivers()
>  pci::pci_device_enumeration();
>  boot_time.event("pci enumerated");
>
> +// Register any parsed virtio-mmio devices
> +virtio::register_mmio_devices(device_manager::instance());
> +
>  // Initialize all drivers
>  hw::driver_manager* drvman = hw::driver_manager::instance();
>  drvman->register_driver(virtio::blk::probe);
> diff --git a/drivers/virtio-mmio.cc b/drivers/virtio-mmio.cc
> new file mode 100644
> index ..181305e0
> --- /dev/null
> +++ b/drivers/virtio-mmio.cc
> @@ -0,0 +1,213 @@
> +//
> +// Created by wkozaczuk on 12/25/18.
> +//
>

The patch looks good, but can you please just replace this with our mode
traditional copyright and license text? Thanks.
(you can then just directly commit this patch yourself, if you want).

+
> +#include 
> +#include 
> +#include "virtio-mmio.hh"
> +
> +namespace virtio {
> +
> +// This implements virtio-io mmio device (transport layer, modeled after
> PSI).
> +// Read here -
> https://www.kraxel.org/virtio/virtio-v1.0-cs03-virtio-gpu.html#x1-1080002
> +hw_device_id mmio_device::get_id()
> +{
> +return hw_device_id(0x1af4, _device_id);
> +}
> +
> +u8 mmio_device::get_status()
> +{
> +return mmio_getl(_addr_mmio + VIRTIO_MMIO_STATUS) & 0xff;
> +}
> +
> +void mmio_device::set_status(u8 status)
> +{
> +mmio_setl(_addr_mmio + VIRTIO_MMIO_STATUS, status);
> +}
> +
> +u64 mmio_device::get_available_features()
> +{
> +u64 features;
> +
> +mmio_setl(_addr_mmio + VIRTIO_MMIO_DEVICE_FEATURES_SEL, 1);
> +features = mmio_getl(_addr_mmio + VIRTIO_MMIO_DEVICE_FEATURES);
> +features <<= 32;
> +
> +mmio_setl(_addr_mmio + VIRTIO_MMIO_DEVICE_FEATURES_SEL, 0);
> +features |= mmio_getl(_addr_mmio + VIRTIO_MMIO_DEVICE_FEATURES);
> +
> +return features;
> +}
> +
> +void mmio_device::set_enabled_features(u64 features)
> +{
> +mmio_setl(_addr_mmio + VIRTIO_MMIO_DRIVER_FEATURES_SEL, 1);
> +mmio_setl(_addr_mmio + VIRTIO_MMIO_DRIVER_FEATURES, (u32)(features >>
> 32));
> +
> +mmio_setl(_addr_mmio + VIRTIO_MMIO_DRIVER_FEATURES_SEL, 0);
> +mmio_setl(_addr_mmio + VIRTIO_MMIO_DRIVER_FEATURES, (u32)features);
> +}
> +
> +void mmio_device::kick_queue(int queue_num)
> +{
> +mmio_setl(_addr_mmio + VIRTIO_MMIO_QUEUE_NOTIFY, queue_num);
> +}
> +
> +void mmio_device::select_queue(int queue_num)
> +{
> +mmio_setl(_addr_mmio + VIRTIO_MMIO_QUEUE_SEL, queue_num);
> +assert(!mmio_getl(_addr_mmio + VIRTIO_MMIO_QUEUE_READY));
> +}
> +
> +u16 mmio_device::get_queue_size()
> +{
> +return mmio_getl(_addr_mmio + VIRTIO_MMIO_QUEUE_NUM_MAX) & 0x;
> +}
> +
> +void mmio_device::setup_queue(vring* queue)
> +{
> +// Set size
> +mmio_setl(_addr_mmio + VIRTIO_MMIO_QUEUE_NUM, queue->size());
> +//
> +// Pass addresses
> +mmio_setl(_addr_mmio + VIRTIO_MMIO_QUEUE_DESC_LOW,
> (u32)queue->get_desc_addr());
> +mmio_setl(_addr_mmio + VIRTIO_MMIO_QUEUE_DESC_HIGH,
> (u32)(queue->get_desc_addr() >> 32));
> +
> +mmio_setl(_addr_mmio + VIRTIO_MMIO_QUEUE_AVAIL_LOW,
> (u32)queue->get_avail_addr());
> +mmio_setl(_addr_mmio + VIRTIO_MMIO_QUEUE_AVAIL_HIGH,
> (u32)(queue->get_avail_addr() >> 32));
> +
> +mmio_setl(_addr_mmio + VIRTIO_MMIO_QUEUE_USED_LOW,
> (u32)queue->get_used_addr());
> +mmio_setl(_addr_mmio + VIRTIO_MMIO_QUEUE_USED_HIGH,
> (u32)(queue->get_used_addr() >> 32));
> +}
> +
> +void mmio_device::activate_queue(int queue)
> +{   //
> +// Make it ready
> +select_queue(queue);
> +mmio_setl(_addr_mmio + 

[osv-dev] [COMMIT osv master] Clean virtio code

2019-03-05 Thread Commit Bot

From: Waldemar Kozaczuk 
Committer: Nadav Har'El 
Branch: master

Clean virtio code

This patch cleans and refines various aspects of virtio
code. More specifically it:
- removes unused methods
- corrects logic to read elements of virtio configuration only
  if corresponding feature bits indicate so
- adds step 6 of modern virtio device initialization
- adds code to register gsi edge interrupt handlers that will
  be used by virtio mmio drivers code in next patch

Signed-off-by: Waldemar Kozaczuk 
Message-Id: <20190302062630.1834-1-jwkozac...@gmail.com>

---
diff --git a/drivers/virtio-blk.cc b/drivers/virtio-blk.cc
--- a/drivers/virtio-blk.cc
+++ b/drivers/virtio-blk.cc
@@ -115,12 +115,11 @@ bool blk::ack_irq()
 blk::blk(virtio_device& virtio_dev)
 : virtio_driver(virtio_dev), _ro(false)
 {
-
 _driver_name = "virtio-blk";
 _id = _instance++;
 virtio_i("VIRTIO BLK INSTANCE %d", _id);

-// Steps 4 & 5 - negotiate and confirm features
+// Steps 4, 5 & 6 - negotiate and confirm features
 setup_features();
 read_config();

@@ -144,6 +143,13 @@ blk::blk(virtio_device& virtio_dev)
 [=] { return this->ack_irq(); },
 [=] { t->wake(); });
 };
+
+int_factory.create_gsi_edge_interrupt = [this,t]() {
+return new gsi_edge_interrupt(
+_dev.get_irq(),
+[=] { if (this->ack_irq()) t->wake(); });
+};
+
 _dev.register_interrupt(int_factory);

 // Enable indirect descriptor
@@ -172,33 +178,34 @@ blk::~blk()
 // including the thread objects and their stack
 }

+#define READ_CONFIGURATION_FIELD(config,field_name,field) \
+virtio_conf_read(offsetof(config,field_name), , sizeof(field));
+
 void blk::read_config()
 {
-if (_dev.is_modern()) {
-//TODO: It may to do with legacy vs non-legacy device
-//but at least with latest spec we should check if individual
-//config fields are available vs reading whole config struct. For  
example

-//firecracker reports memory read violation warnings
-virtio_conf_read(0, &_config, sizeof(_config.capacity));
-}
-else {
-//read all of the block config (including size, mce, topology,..)  
in one shot

-virtio_conf_read(0, &_config, sizeof(_config));
-}
-
+READ_CONFIGURATION_FIELD(blk_config,capacity,_config.capacity)
 trace_virtio_blk_read_config_capacity(_config.capacity);

-if (get_guest_feature_bit(VIRTIO_BLK_F_SIZE_MAX))
+if (get_guest_feature_bit(VIRTIO_BLK_F_SIZE_MAX)) {
+READ_CONFIGURATION_FIELD(blk_config,size_max,_config.size_max)
 trace_virtio_blk_read_config_size_max(_config.size_max);
-if (get_guest_feature_bit(VIRTIO_BLK_F_SEG_MAX))
+}
+if (get_guest_feature_bit(VIRTIO_BLK_F_SEG_MAX)) {
+READ_CONFIGURATION_FIELD(blk_config,seg_max,_config.seg_max)
 trace_virtio_blk_read_config_seg_max(_config.seg_max);
+}
 if (get_guest_feature_bit(VIRTIO_BLK_F_GEOMETRY)) {
+READ_CONFIGURATION_FIELD(blk_config,geometry,_config.geometry)
  
trace_virtio_blk_read_config_geometry((u32)_config.geometry.cylinders,  
(u32)_config.geometry.heads, (u32)_config.geometry.sectors);

 }
-if (get_guest_feature_bit(VIRTIO_BLK_F_BLK_SIZE))
+if (get_guest_feature_bit(VIRTIO_BLK_F_BLK_SIZE)) {
+READ_CONFIGURATION_FIELD(blk_config,blk_size,_config.blk_size)
 trace_virtio_blk_read_config_blk_size(_config.blk_size);
+}
 if (get_guest_feature_bit(VIRTIO_BLK_F_TOPOLOGY)) {
- 
trace_virtio_blk_read_config_topology((u32)_config.physical_block_exp,  
(u32)_config.alignment_offset, (u32)_config.min_io_size,  
(u32)_config.opt_io_size);

+READ_CONFIGURATION_FIELD(blk_config,topology,_config.topology)
+ 
trace_virtio_blk_read_config_topology((u32)_config.topology.physical_block_exp,  
(u32)_config.topology.alignment_offset,
+  (u32)_config.topology.min_io_size,  
(u32)_config.topology.opt_io_size);

 }
 if (get_guest_feature_bit(VIRTIO_BLK_F_CONFIG_WCE))
 trace_virtio_blk_read_config_wce((u32)_config.wce);
@@ -260,8 +267,7 @@ int blk::make_request(struct bio* bio)

 if (!bio) return EIO;

-// TODO: Check if seg_max is unavailable if modern ...
-if (!_dev.is_modern()) {
+if (get_guest_feature_bit(VIRTIO_BLK_F_SEG_MAX)) {
 if (bio->bio_bcount/mmu::page_size + 1 > _config.seg_max) {
 trace_virtio_blk_make_request_seg_max(bio->bio_bcount,  
_config.seg_max);

 return EIO;
diff --git a/drivers/virtio-blk.hh b/drivers/virtio-blk.hh
--- a/drivers/virtio-blk.hh
+++ b/drivers/virtio-blk.hh
@@ -82,15 +82,17 @@ public:
 /* block size of device (if VIRTIO_BLK_F_BLK_SIZE) */
 u32 blk_size;

-/* the next 4 entries are guarded by VIRTIO_BLK_F_TOPOLOGY  */
-/* exponent for physical block per logical block. */
-u8 physical_block_exp;
-/*