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 <[email protected] > <javascript:>> 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: <Software, Yarrow> 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: 0xffffa00000f16f00 > 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 (https://github.com/firecracker-microvm/firecracker/blob/master/docs/design.md#layout - single key?) - I am not sure we can make it better. So I think all interactive apps will never properly work on firecracker. But I might be completely wrong. Not very familiar with keyboard handling. > > >> 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 <[email protected] <javascript:>> >>> --- >>> 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 = &c->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 = &c->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<uint32_t *>(addr) == MPF_IDENTIFIER) { >>> + // We found the MP floating pointer structure >>> + auto mpf_struct = static_cast<mpf_structure*>(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<uint32_t *>(mp_table_addr) == >>> MP_TABLE_IDENTIFIER) { >>> + return static_cast<mp_table*>(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 = &c->arch.initstack; >>> - sched::cpus.push_back(c); >>> + // Parse information about all vCPUs from MP table. For details >>> please see >>> + // >>> https://wiki.osdev.org/Symmetric_Multiprocessing#Finding_information_using_MP_Table >>> >>> + // or >>> http://www.osdever.net/tutorials/view/multiprocessing-support-for-hobby-oses-explained >>> >>> + mp_table *table = find_mp_table(LAST_KB_IN_BASE_MEMORY_ADDR, >>> 0x400); >>> + if (!table) { >>> + table = find_mp_table(FIRST_KB_IN_BASE_MEMORY_ADDR, 0x400); >>> + } >>> + >>> + unsigned nr_cpus = 0; >>> + if (table) { >>> + void *mp_entries = static_cast<void*>(table) + >>> sizeof(mp_table); >>> + int entries_size = table->length - sizeof(mp_table); >>> + >>> + while (entries_size > 0) { >>> + int entry_size = NON_PROCESSOR_ENTRY_SIZE; >>> + auto proc_desc = static_cast<mp_processor*>(mp_entries); >>> + if (proc_desc->type == 0) { >>> + register_cpu(nr_cpus++, proc_desc->local_apic_id); >>> + entry_size = sizeof(mp_processor); >>> + } >>> + entries_size -= entry_size; >>> + mp_entries += entry_size; >>> + } >>> + } >>> + >>> + if (!nr_cpus) { // No MP table was found or no cpu was found in >>> there -> assume uni-processor >>> + register_cpu(nr_cpus++, 0); >>> + } >>> + >>> + debug(fmt("%d CPUs detected\n") % nr_cpus); >>> } >>> >>> void smp_init() >>> -- >>> 2.19.1 >>> >>> -- >> 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 [email protected] <javascript:>. >> For more options, visit https://groups.google.com/d/optout. >> > -- 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 [email protected]. For more options, visit https://groups.google.com/d/optout.
