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 <[email protected]> >> 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. > After I wrote this I remembered that when I booted linux on firecracker I was able to type stuff in which means that keyboard is supported pretty well. So indeed the issue might be with the firecracker.py.
> >> >>> 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]> >>>> --- >>>> 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]. >>> 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.
