I just wanted to get a sense if my code is logically correct. So what you looked at is not "reviewable" yet. I am planning to refactor and send proper patches next week.
The hyperv_clocksource is a new flag that I added to to features in my branch. I gets set if cpuid logic detects if OSv is running on Hyper/V. The hyperv_tcf64_rdmsr() is a function in original FreeBSD Hyper/V drivers source that I ported. On Fri, May 19, 2017 at 4:58 PM, Nadav Har'El <n...@scylladb.com> wrote: > Looks reasonable. I don't see all your patches so I can't make a complete > review. In particular where is hyperv_tc64_rdmsr defined? Why is it not > in the this same source file, or if it's in a different one, why is it not > declared in a header file? > > I know you copied this from hpet.c, but I would use the name _boot_wall > instead of just "_wall". > > Where does processor::features().hyperv_clocksource come from? Does this > check the cpuid bits saying if hyperv_tcf64_rdmsr is available? Or does it > check something else? > > By the way, I just noticed that HyperV apparently also has something calls > a "reference TSC page" which is very similar to the existing kvm (and xen) > paravirtual clock, so we can probably use that when you want. Seems Linux > added support for this just last year: https://patchwork.kernel.org/ > patch/8902961/. But maybe you'll want to add this as a "TODO" and only go > back to it after you make the network (for example) work. > > > -- > Nadav Har'El > n...@scylladb.com > > On Fri, May 19, 2017 at 6:03 PM, Waldek Kozaczuk <jwkozac...@gmail.com> > wrote: > >> The essence of my changes is this new clock class - >> https://github.com/wkozaczuk/osv/blob/hyperv_vmbus/drivers/hypervclock.cc. >> Let me know if this implementation is fundamentally flawed or overly naive. >> >> >> On Friday, May 19, 2017 at 12:52:55 AM UTC-4, Waldek Kozaczuk wrote: >>> >>> I finally managed to fully boot and execute simple C hello work program >>> on Hyper/V. >>> >>> It involved adding simple logic to detect if OSv is running on Hyper/V >>> and implementing simple hyperv clock modeled after drivers/hpet.cc which >>> uses RTC to grab wall clock time and Partition Reference Counter MSR >>> (section 12.4 from https://github.com/Micros >>> oft/Virtualization-Documentation/raw/master/tlfs/Hypervisor% >>> 20Top%20Level%20Functional%20Specification%20v5.0b.pdf) as a TSC source. >>> >>> I am not sure if my implementation is most efficient and fully correct >>> but it seems to do its job. >>> >>> I will be sending number of patches soon. >>> >>> Regards, >>> Waldek >>> >>> On Monday, May 15, 2017 at 10:08:20 AM UTC-4, Nadav Har'El wrote: >>>> >>>> >>>> On Fri, May 12, 2017 at 10:26 PM, Waldek Kozaczuk <jwkoz...@gmail.com> >>>> wrote: >>>> >>>>> Given I am almost done porting the FreeBSD drivers code for Hyper/V >>>>> (at least the VMBus part) I have decided to proceed with the >>>>> implementation >>>>> of the clock using Hyper/V time synchronization service. This will >>>>> obviously require putting enough "glue" between the original BSD drivers >>>>> code and OSv core in form of new classes in drivers/ folder like >>>>> hyperv::vmbus >>>>> (modelled after xen::xenbus) and possibly other ones. I will still try to >>>>> minimize it so I can get the hello work program to fully execute. >>>>> >>>> >>>> Ok, good luck. >>>> >>>> >>>>> I also wanted to ask your advice about approach I should take >>>>> regarding the organization of patches that I would be eventually sending >>>>> at >>>>> some point. I was thinking that my work should be divided in the series of >>>>> patches. The first one would be a patch with original FreeBSD drivers code >>>>> and second one would be a patch with all the changes related to porting >>>>> the >>>>> code to C++ and when necessary adapting it to OSv APIs. >>>>> >>>> >>>> That sounds good, if the changes from the BSD driver are small enough >>>> to be reviewable (the reviewer, e.g. me, will likely not be familiar with >>>> the freebsd code). >>>> >>>> >>>>> That way I hoped it would make it easy to review to see how I changed >>>>> the original FreeBSD code. Following patches would focus on building the >>>>> "glue". In general how incremental these patches should be in your >>>>> opinion? >>>>> >>>> >>>> In general, if these patches are implementing support for a completely >>>> new hypervisor, and mostly adding new source files, they don't have to be >>>> split up very finely. If you're modify existing code, please do send those >>>> modifications as separate patches. >>>> >>>> Nadav. >>>> >>> -- >> 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. >> > > -- 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.
